diff for review

[Simon TAYLOR]

Hi Fergus,

Could you please review this,

Simon.


Estimated hours taken: 3

Bug fixes

compiler/simplify.m
	Fix a code generator abort for if-then-elses with conditions
	that cannot succeed by fixing and enabling the code to optimise
	away such cases.
	Stop warnings appearing multiple times, make sure the warnings
	appear in the correct order.

tests/warnings/Mmake
tests/warnings/simple_code.{m, exp}
	Test cases for the warnings from simplify.m

tests/valid/Mmake
tests/valid/fail_ite.m
	Regression test for the abort.



Index: simplify.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/simplify.m,v
retrieving revision 1.29
diff -u -r1.29 simplify.m
--- simplify.m	1997/04/08 02:26:34	1.29
+++ simplify.m	1997/04/14 23:32:37
@@ -80,9 +80,8 @@
 		VarSet0, VarTypes0, Info0),
 	write_pred_progress_message("% Simplifying ", PredId, ModuleInfo0,
 		State1, State2),
+	Simplify = simplify(Warn, WarnCalls, Once, Switch, _, Excess, Calls),
 	( simplify_do_common(Info0) ->
-		Simplify = simplify(Warn, WarnCalls, Once, 
-				Switch, _, Excess, Calls),
 		% On the first pass do common structure elimination and
 		% branch merging.
 		simplify_info_set_simplify(Info0,
@@ -94,7 +93,7 @@
 		proc_info_variables(Proc1, VarSet1),
 		proc_info_vartypes(Proc1, VarTypes1),
 		simplify_info_init(DetInfo,
-			simplify(Warn, WarnCalls, Once, no, no, Excess, no),
+			simplify(no, no, Once, no, no, Excess, no),
 			InstMap0, VarSet1, VarTypes1, Info3),
 		simplify_info_set_msgs(Info3, Msgs1, Info4),
 		%proc_info_goal(Proc1, OutGoal),
@@ -113,8 +112,8 @@
 	simplify__proc_2(Proc1, Proc, ModuleInfo1, ModuleInfo,
 			Info4, Info, State4, State5),
 	simplify_info_get_msgs(Info, Msgs2),
-	set__to_sorted_list(Msgs2, Msgs),
-	( simplify_do_warn(Info) ->
+	list__reverse(Msgs2, Msgs),
+	( (Warn = yes ; WarnCalls = yes) ->
 		det_report_msgs(Msgs, ModuleInfo, WarnCnt,
 			ErrCnt, State5, State)
 	;
@@ -472,40 +471,46 @@
 	% is finished, or when we start doing coroutining.
 
 simplify__goal_2(if_then_else(Vars, Cond0, Then0, Else0, SM),
-		GoalInfo, Goal, GoalInfo, Info0, Info) :-
+		GoalInfo0, Goal, GoalInfo, Info0, Info) :-
 	Cond0 = _ - CondInfo0,
 
 	goal_info_get_determinism(CondInfo0, CondDetism),
-	determinism_components(CondDetism, CondCanFail, _CondSolns),
+	determinism_components(CondDetism, CondCanFail, CondSolns),
 	( CondCanFail = cannot_fail ->
 		goal_to_conj_list(Cond0, CondList),
 		goal_to_conj_list(Then0, ThenList),
 		list__append(CondList, ThenList, List),
-		simplify__goal(conj(List) - GoalInfo, Goal - _,
+		simplify__goal(conj(List) - GoalInfo0, Goal - GoalInfo,
 			Info0, Info1),
 		simplify_info_add_msg(Info1, ite_cond_cannot_fail(GoalInfo),
 			Info)
-/*********
-	The following optimization is disabled, because it is
-	buggy (see the XXX below).  It's not important, since
-	most of these cases will be optimized by modes.m anyway.
 	; CondSolns = at_most_zero ->
 		% Optimize away the condition and the `then' part.
+		goal_info_get_determinism(CondInfo0, Detism),
+		det_negation_det(Detism, MaybeNegDetism),
+		(
+			MaybeNegDetism = yes(NegDetism)
+		;
+			MaybeNegDetism = no,
+			error("simplify__goal_2: cannot get negated determinism")
+		),
+		goal_info_set_determinism(CondInfo0, NegDetism, NegCondInfo0),
+		instmap_delta_init_reachable(NegInstMapDelta),
+		goal_info_set_instmap_delta(NegCondInfo0, 
+			NegInstMapDelta, NegCondInfo),
 		goal_to_conj_list(Else0, ElseList),
-		% XXX Using CondInfo without updating the determinism is a bug.
-		% We should probably update other goal_info fields as well,
-		% e.g. the instmap_delta.
-		List = [not(Cond0) - CondInfo | ElseList],
-		simplify__goal(conj(List) - GoalInfo, InstMap0, DetInfo,
-			Goal - _, Msgs1),
-		Msgs = [ite_cond_cannot_succeed(GoalInfo) | Msgs1]
-**********/
+		List = [not(Cond0) - NegCondInfo | ElseList],
+		simplify__goal(conj(List) - GoalInfo0, Goal - GoalInfo,
+			Info0, Info1),
+		simplify_info_add_msg(Info1, ite_cond_cannot_succeed(GoalInfo),
+			Info)
 	; Else0 = disj([], _) - _ ->
 		% (A -> C ; fail) is equivalent to (A, C)
 		goal_to_conj_list(Cond0, CondList),
 		goal_to_conj_list(Then0, ThenList),
 		list__append(CondList, ThenList, List),
-		simplify__goal(conj(List) - GoalInfo, Goal - _, Info0, Info)
+		simplify__goal(conj(List) - GoalInfo0, Goal - GoalInfo,
+			Info0, Info)
 	;
 		simplify__goal(Cond0, Cond, Info0, Info1),
 		simplify_info_update_instmap(Info1, Cond, Info2),
@@ -523,7 +528,8 @@
 		goal_info_get_instmap_delta(ElseInfo, ElseDelta),
 		simplify_info_create_branch_info(Info0, Info6,
 			[ElseDelta, CondThenDelta], Info),
-		Goal = if_then_else(Vars, Cond, Then, Else, SM)
+		Goal = if_then_else(Vars, Cond, Then, Else, SM),
+		GoalInfo = GoalInfo0
 	).
 
 simplify__goal_2(not(Goal0), GoalInfo, Goal, GoalInfo, Info0, Info) :-
@@ -1115,7 +1121,7 @@
 :- type simplify_info
 	--->	simplify_info(
 			det_info,
-			set(det_msg),
+			list(det_msg),
 			simplify,	% How much simplification to do.
 			common_info,	% Info about common subexpressions.
 			instmap,
@@ -1139,16 +1145,15 @@
 		).
 
 simplify_info_init(DetInfo, Simplify, InstMap, VarSet, VarTypes, Info) :-
-	set__init(Msgs),
 	common_info_init(CommonInfo),
-	Info = simplify_info(DetInfo, Msgs, Simplify, CommonInfo,
+	Info = simplify_info(DetInfo, [], Simplify, CommonInfo,
 			InstMap, VarSet, VarTypes, no, no, no). 
 
 	% exported for common.m
 :- interface.
 
 :- pred simplify_info_get_det_info(simplify_info::in, det_info::out) is det.
-:- pred simplify_info_get_msgs(simplify_info::in, set(det_msg)::out) is det.
+:- pred simplify_info_get_msgs(simplify_info::in, list(det_msg)::out) is det.
 :- pred simplify_info_get_instmap(simplify_info::in, instmap::out) is det.
 :- pred simplify_info_get_simplify(simplify_info::in, simplify::out) is det.
 :- pred simplify_info_get_common_info(simplify_info::in,
@@ -1192,7 +1197,7 @@
 :- pred simplify_info_set_det_info(simplify_info::in,
 		det_info::in, simplify_info::out) is det.
 :- pred simplify_info_set_msgs(simplify_info::in,
-		set(det_msg)::in, simplify_info::out) is det.
+		list(det_msg)::in, simplify_info::out) is det.
 :- pred simplify_info_set_simplify(simplify_info::in,
 		simplify::in, simplify_info::out) is det.
 :- pred simplify_info_set_instmap(simplify_info::in,
@@ -1245,8 +1250,7 @@
 simplify_info_add_msg(Info0, Msg, Info) :-
 	( simplify_do_warn(Info0) ->
 		simplify_info_get_msgs(Info0, Msgs0),
-		set__insert(Msgs0, Msg, Msgs),
-		simplify_info_set_msgs(Info0, Msgs, Info)
+		simplify_info_set_msgs(Info0, [Msg | Msgs0], Info)
 	;
 		Info = Info0
 	).


tests/warnings/simple_code.m:

:- module simple_code.
:- interface.
:- import_module io.
:- pred p(int::in, int::out) is det.
:- implementation.
:- import_module require.
p --> 
	
	(
		[]
	;
		{ error("foo") }
	),
	( { true } ->
		{ Z = 2 }	
	;
		{ Z = 3 }
	),
	( { X = 3, X = 2, Z = 2 } ->
		[]
	;
		[]
	),
	( { \+ true } ->
		[]
	;
		[]
	),
	( { \+ det_pred } ->
		[]
	;
		[]
	),
	( { \+ fail_pred } ->
		[]
	;
		[]
	),
	{ \+ fail },
	{ obsolete }.

:- pred det_pred is det.

det_pred.

:- pred fail_pred is failure.

fail_pred :- fail.

:- pred obsolete is det.
:- pragma obsolete(obsolete/0).

obsolete.


tests/warnings/simple_code.exp:

simple_code.m:011: Warning: this disjunct will never have any solutions.
simple_code.m:016: Warning: the condition of this if-then-else cannot fail.
simple_code.m:026: Warning: the condition of this if-then-else cannot succeed.
simple_code.m:031: Warning: the condition of this if-then-else cannot succeed.
simple_code.m:036: Warning: the condition of this if-then-else cannot fail.
simple_code.m:040: Warning: call to obsolete predicate `simple_code:obsolete/0'.




tests/valid/fail_ite.m:

:- module fail_ite.
:- interface.
:- import_module io.
:- pred p(io__state::di, io__state::uo) is det.
:- implementation.
:- import_module require.
p --> 
	( { \+ det_pred } ->
		[]
	;
		[]
	),
	( { \+ fail_pred } ->
		[]
	;
		[]
	).

:- pred det_pred is det.

det_pred.

:- pred fail_pred is failure.

fail_pred :- fail.