diff: Better error reporting for illegally bound vars

[Andrew Bromage]

G'day all.

Fergus, could you please review this?  Thanks.

While you're at it, should the locked_vars type be made abstract?

Cheers,
Andrew Bromage


Estimated hours taken: 2

Better error reporting for variables bound in illegal places.

compiler/mode_info.m:
	Add a var_lock_reason, specifying the reason why a variable
	is locked during mode checking.

compiler/modecheck_unify.m:
compiler/modes.m:
compiler/unique_modes.m:
	Specify the reason when locking some variables.

compiler/mode_errors.m:
	When reporting var locking errors, give the reason why the
	var was locked.

tests/invalid/Mmakefile:
tests/invalid/bind_var_errors.err_exp:
tests/invalid/bind_var_errors.m:
	Regression tests for the above changes.


Index: compiler/mode_errors.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/mode_errors.m,v
retrieving revision 1.53
diff -u -r1.53 mode_errors.m
--- mode_errors.m	1998/01/23 12:56:46	1.53
+++ mode_errors.m	1998/01/27 07:21:53
@@ -65,7 +65,7 @@
 	;	mode_error_no_matching_mode(list(var), list(inst))
 			% call to a predicate with an insufficiently
 			% instantiated variable (for preds with >1 mode)
-	;	mode_error_bind_var(var, inst, inst)
+	;	mode_error_bind_var(var_lock_reason, var, inst, inst)
 			% attempt to bind a non-local variable inside
 			% a negated context
 	;	mode_error_unify_var_var(var, var, inst, inst)
@@ -188,8 +188,8 @@
 	report_mode_error_implied_mode(ModeInfo, Var, InstA, InstB).
 report_mode_error(mode_error_no_mode_decl, ModeInfo) -->
 	report_mode_error_no_mode_decl(ModeInfo).
-report_mode_error(mode_error_bind_var(Var, InstA, InstB), ModeInfo) -->
-	report_mode_error_bind_var(ModeInfo, Var, InstA, InstB).
+report_mode_error(mode_error_bind_var(Reason, Var, InstA, InstB), ModeInfo) -->
+	report_mode_error_bind_var(ModeInfo, Reason, Var, InstA, InstB).
 report_mode_error(mode_error_unify_var_var(VarA, VarB, InstA, InstB),
 		ModeInfo) -->
 	report_mode_error_unify_var_var(ModeInfo, VarA, VarB, InstA, InstB).
@@ -367,18 +367,29 @@
 
 %-----------------------------------------------------------------------------%
 
-:- pred report_mode_error_bind_var(mode_info, var, inst, inst,
+:- pred report_mode_error_bind_var(mode_info, var_lock_reason, var, inst, inst,
 					io__state, io__state).
-:- mode report_mode_error_bind_var(mode_info_ui, in, in, in, di, uo) is det.
+:- mode report_mode_error_bind_var(mode_info_ui, in, in, in, in, di, uo) is det.
 
-report_mode_error_bind_var(ModeInfo, Var, VarInst, Inst) -->
+report_mode_error_bind_var(ModeInfo, Reason, Var, VarInst, Inst) -->
 	{ mode_info_get_context(ModeInfo, Context) },
 	{ mode_info_get_varset(ModeInfo, VarSet) },
 	{ mode_info_get_instvarset(ModeInfo, InstVarSet) },
 	mode_info_write_context(ModeInfo),
 	prog_out__write_context(Context),
-	io__write_string(
-		"  scope error: attempt to bind variable inside a negation.\n"),
+	io__write_string("  scope error: "),
+	( { Reason = negation },
+		io__write_string("attempt to bind a variable inside a negation.\n")
+	; { Reason = if_then_else },
+		io__write_string("attempt to bind a non-local variable inside the\n"),
+		prog_out__write_context(Context),
+		io__write_string("  condition of an if-then-else.\n")
+	; { Reason = lambda(PredOrFunc) },
+		{ hlds_out__pred_or_func_to_str(PredOrFunc, PredOrFuncS) },
+		io__write_string("attempt to bind a non-local variable inside\n"),
+		prog_out__write_context(Context),
+		io__write_strings(["  a ", PredOrFuncS, " lambda goal.\n"])
+	),
 	prog_out__write_context(Context),
 	io__write_string("  Variable `"),
 	mercury_output_var(Var, VarSet, no),
@@ -391,12 +402,18 @@
 	io__write_string("'.\n"),
 	globals__io_lookup_bool_option(verbose_errors, VerboseErrors),
 	( { VerboseErrors = yes } ->
-		io__write_string("\tA negation is only allowed to bind variables which are local to the\n"),
-		io__write_string("\tnegation, i.e. those which are implicitly existentially quantified\n"),
-		io__write_string("\tinside the scope of the negation.\n"),
-		io__write_string("\tNote that the condition of an if-then-else is implicitly\n"),
-		io__write_string("\tnegated in the ""else"" part, so the condition can only bind\n"),
-		io__write_string("\tvariables in the ""then"" part.\n")
+		( { Reason = negation },
+			io__write_string("\tA negation is only allowed to bind variables which are local to the\n"),
+			io__write_string("\tnegation, i.e. those which are implicitly existentially quantified\n"),
+			io__write_string("\tinside the scope of the negation.\n")
+		; { Reason = if_then_else },
+			io__write_string("\tThe condition of an if-then-else is only allowed\n"),
+			io__write_string("\tto bind variables which are local to the condition\n"),
+			io__write_string("\tor which occur only in the condition and the `then' part.\n")
+		; { Reason = lambda(_) },
+			io__write_string("\tA lambda goal is only allowed to bind its arguments\n"),
+			io__write_string("\tand variables local to the lambda expression.\n")
+		)
 	;
 		[]
 	).
Index: compiler/mode_info.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/mode_info.m,v
retrieving revision 1.43
diff -u -r1.43 mode_info.m
--- mode_info.m	1998/01/21 06:10:59	1.43
+++ mode_info.m	1998/01/27 07:00:52
@@ -56,6 +56,13 @@
 	;	call(pred_id)
 	;	higher_order_call(pred_or_func).
 
+:- type var_lock_reason
+	--->	negation
+	;	if_then_else
+	;	lambda(pred_or_func).
+
+:- type locked_vars == assoc_list(var_lock_reason, set(var)).
+
 :- type mode_info.
 
 :- pred mode_info_init(io__state, module_info, pred_id, proc_id,
@@ -119,10 +126,10 @@
 :- pred mode_info_set_instmap(instmap, mode_info, mode_info).
 :- mode mode_info_set_instmap(in, mode_info_di, mode_info_uo) is det.
 
-:- pred mode_info_get_locked_vars(mode_info, list(set(var))).
+:- pred mode_info_get_locked_vars(mode_info, locked_vars).
 :- mode mode_info_get_locked_vars(mode_info_ui, out) is det.
 
-:- pred mode_info_set_locked_vars(mode_info, list(set(var)), mode_info).
+:- pred mode_info_set_locked_vars(mode_info, locked_vars, mode_info).
 :- mode mode_info_set_locked_vars(mode_info_di, in, mode_info_uo) is det.
 
 :- pred mode_info_get_errors(mode_info, list(mode_error_info)).
@@ -173,17 +180,17 @@
 :- pred mode_info_get_types_of_vars(mode_info, list(var), list(type)).
 :- mode mode_info_get_types_of_vars(mode_info_ui, in, out) is det.
 
-:- pred mode_info_lock_vars(set(var), mode_info, mode_info).
-:- mode mode_info_lock_vars(in, mode_info_di, mode_info_uo) is det.
+:- pred mode_info_lock_vars(var_lock_reason, set(var), mode_info, mode_info).
+:- mode mode_info_lock_vars(in, in, mode_info_di, mode_info_uo) is det.
 
-:- pred mode_info_unlock_vars(set(var), mode_info, mode_info).
-:- mode mode_info_unlock_vars(in, mode_info_di, mode_info_uo) is det.
+:- pred mode_info_unlock_vars(var_lock_reason, set(var), mode_info, mode_info).
+:- mode mode_info_unlock_vars(in, in, mode_info_di, mode_info_uo) is det.
 
-:- pred mode_info_var_is_locked(mode_info, var).
-:- mode mode_info_var_is_locked(mode_info_ui, in) is semidet.
+:- pred mode_info_var_is_locked(mode_info, var, var_lock_reason).
+:- mode mode_info_var_is_locked(mode_info_ui, in, out) is semidet.
 
-:- pred mode_info_var_is_locked_2(list(set(var)), var).
-:- mode mode_info_var_is_locked_2(in, in) is semidet.
+:- pred mode_info_var_is_locked_2(locked_vars, var, var_lock_reason).
+:- mode mode_info_var_is_locked_2(in, in, out) is semidet.
 
 :- pred mode_info_get_delay_info(mode_info, delay_info).
 :- mode mode_info_get_delay_info(mode_info_no_io, out) is det.
@@ -285,7 +292,7 @@
 					% goal the error occurred
 			instmap,	% The current instantiatedness
 					% of the variables
-			list(set(var)),	% The "locked" variables,
+			locked_vars,	% The "locked" variables,
 					% i.e. variables which cannot be
 					% further instantiated inside a
 					% negated context
@@ -633,30 +640,34 @@
 	% push them on the stack, and to unlock a set of vars, we just
 	% pop them off the stack.  The stack is implemented as a list.
 
-mode_info_lock_vars(Vars, ModeInfo0, ModeInfo) :-
+mode_info_lock_vars(Reason, Vars, ModeInfo0, ModeInfo) :-
 	mode_info_get_locked_vars(ModeInfo0, LockedVars),
-	mode_info_set_locked_vars(ModeInfo0, [Vars | LockedVars], ModeInfo).
+	mode_info_set_locked_vars(ModeInfo0, [Reason - Vars | LockedVars],
+			ModeInfo).
 
-mode_info_unlock_vars(_, ModeInfo0, ModeInfo) :-
+mode_info_unlock_vars(Reason, Vars, ModeInfo0, ModeInfo) :-
 	mode_info_get_locked_vars(ModeInfo0, LockedVars0),
-	( LockedVars0 = [_ | LockedVars1] ->
+	(
+		LockedVars0 = [Reason - TheseVars | LockedVars1],
+		set__equal(TheseVars, Vars)
+	->
 		LockedVars = LockedVars1
 	;
-		error("mode_info_unlock_vars: stack is empty")
+		error("mode_info_unlock_vars: some kind of nesting error")
 	),
 	mode_info_set_locked_vars(ModeInfo0, LockedVars, ModeInfo).
 
-mode_info_var_is_locked(ModeInfo, Var) :-
+mode_info_var_is_locked(ModeInfo, Var, Reason) :-
 	mode_info_get_locked_vars(ModeInfo, LockedVarsList),
-	mode_info_var_is_locked_2(LockedVarsList, Var).
+	mode_info_var_is_locked_2(LockedVarsList, Var, Reason).
 
-mode_info_var_is_locked_2([Set | Sets], Var) :-
+mode_info_var_is_locked_2([ThisReason - Set | Sets], Var, Reason) :-
 	(
 		set__member(Var, Set)
 	->
-		true
+		Reason = ThisReason
 	;
-		mode_info_var_is_locked_2(Sets, Var)
+		mode_info_var_is_locked_2(Sets, Var, Reason)
 	).
 
 mode_info_get_delay_info(mode_info(_,_,_,_,_,_,_,_,_,_,DelayInfo,_,_,_,_,_),
Index: compiler/modecheck_unify.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/modecheck_unify.m,v
retrieving revision 1.26
diff -u -r1.26 modecheck_unify.m
--- modecheck_unify.m	1998/01/16 06:44:39	1.26
+++ modecheck_unify.m	1998/01/27 06:21:20
@@ -384,7 +384,8 @@
 	Goal0 = _ - GoalInfo0,
 	goal_info_get_nonlocals(GoalInfo0, NonLocals0),
 	set__delete_list(NonLocals0, Vars, NonLocals),
-	mode_info_lock_vars(NonLocals, ModeInfo2, ModeInfo3),
+	mode_info_lock_vars(lambda(PredOrFunc), NonLocals,
+		ModeInfo2, ModeInfo3),
  
 	mode_checkpoint(enter, "lambda goal", ModeInfo3, ModeInfo4),
 	% if we're being called from unique_modes.m, then we need to 
@@ -399,7 +400,8 @@
 	mode_checkpoint(exit, "lambda goal", ModeInfo6, ModeInfo7),
  
 	mode_info_remove_live_vars(LiveVars, ModeInfo7, ModeInfo8),
-	mode_info_unlock_vars(NonLocals, ModeInfo8, ModeInfo9),
+	mode_info_unlock_vars(lambda(PredOrFunc), NonLocals,
+		ModeInfo8, ModeInfo9),
 	mode_info_set_instmap(InstMap0, ModeInfo9, ModeInfo10),
  
 	%
Index: compiler/modes.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/modes.m,v
retrieving revision 1.217
diff -u -r1.217 modes.m
--- modes.m	1998/01/24 05:44:21	1.217
+++ modes.m	1998/01/27 06:35:01
@@ -894,11 +894,11 @@
 	{ goal_info_get_nonlocals(GoalInfo0, NonLocals) },
 	{ goal_get_nonlocals(B0, B_Vars) },
 	mode_info_dcg_get_instmap(InstMap0),
-	mode_info_lock_vars(NonLocals),
+	mode_info_lock_vars(if_then_else, NonLocals),
 	mode_info_add_live_vars(B_Vars),
 	modecheck_goal(A0, A),
 	mode_info_remove_live_vars(B_Vars),
-	mode_info_unlock_vars(NonLocals),
+	mode_info_unlock_vars(if_then_else, NonLocals),
 	modecheck_goal(B0, B),
 	mode_info_dcg_get_instmap(InstMapB),
 	mode_info_set_instmap(InstMap0),
@@ -913,9 +913,9 @@
 	mode_checkpoint(enter, "not"),
 	{ goal_info_get_nonlocals(GoalInfo0, NonLocals) },
 	mode_info_dcg_get_instmap(InstMap0),
-	mode_info_lock_vars(NonLocals),
+	mode_info_lock_vars(negation, NonLocals),
 	modecheck_goal(A0, A),
-	mode_info_unlock_vars(NonLocals),
+	mode_info_unlock_vars(negation, NonLocals),
 	mode_info_set_instmap(InstMap0),
 	mode_checkpoint(exit, "not").
 
@@ -1582,12 +1582,12 @@
 		;
 			% We've bound part of the var.  If the var was locked,
 			% then we need to report an error.
-			mode_info_var_is_locked(ModeInfo1, Var0)
+			mode_info_var_is_locked(ModeInfo1, Var0, Reason0)
 		->
 			set__singleton_set(WaitingVars, Var0),
 			mode_info_error(WaitingVars,
-					mode_error_bind_var(Var0, Inst0, Inst),
-					ModeInfo1, ModeInfo
+				mode_error_bind_var(Reason0, Var0, Inst0, Inst),
+				ModeInfo1, ModeInfo
 			)
 		;
 			instmap__set(InstMap0, Var0, Inst, InstMap),
Index: compiler/unique_modes.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/unique_modes.m,v
retrieving revision 1.44
diff -u -r1.44 unique_modes.m
--- unique_modes.m	1998/01/13 10:13:55	1.44
+++ unique_modes.m	1998/01/27 06:35:10
@@ -294,7 +294,7 @@
 	{ unique_modes__goal_get_nonlocals(B0, B_Vars) },
 	{ unique_modes__goal_get_nonlocals(C0, C_Vars) },
 	mode_info_dcg_get_instmap(InstMap0),
-	mode_info_lock_vars(NonLocals),
+	mode_info_lock_vars(if_then_else, NonLocals),
 
 	%
 	% At this point, we should set the inst of any `unique'
@@ -320,7 +320,7 @@
 	mode_info_add_live_vars(B_Vars),
 	unique_modes__check_goal(A0, A),
 	mode_info_remove_live_vars(B_Vars),
-	mode_info_unlock_vars(NonLocals),
+	mode_info_unlock_vars(if_then_else, NonLocals),
 	% mode_info_dcg_get_instmap(InstMapA),
 	unique_modes__check_goal(B0, B),
 	mode_info_dcg_get_instmap(InstMapB),
@@ -340,9 +340,9 @@
 	=(ModeInfo),
 	{ select_live_vars(NonLocalsList, ModeInfo, LiveNonLocals) },
 	make_var_list_mostly_uniq(LiveNonLocals),
-	mode_info_lock_vars(NonLocals),
+	mode_info_lock_vars(negation, NonLocals),
 	unique_modes__check_goal(A0, A),
-	mode_info_unlock_vars(NonLocals),
+	mode_info_unlock_vars(negation, NonLocals),
 	mode_info_set_instmap(InstMap0),
 	mode_checkpoint(exit, "not").
 
Index: tests/invalid/Mmakefile
===================================================================
RCS file: /home/staff/zs/imp/tests/invalid/Mmakefile,v
retrieving revision 1.9
diff -u -r1.9 Mmakefile
--- Mmakefile	1998/01/12 13:30:00	1.9
+++ Mmakefile	1998/01/27 07:20:07
@@ -9,6 +9,7 @@
 SOURCES= \
 	any_mode.m \
 	bigtest.m \
+	bind_var_errors.m \
 	circ_type.m \
 	constructor_warning.m \
 	det_errors.m \

New File: tests/invalid/bind_var_errors.err_exp
===================================================================
bind_var_errors.m:031: In clause for `bind_var_in_negation':
bind_var_errors.m:031:   scope error: attempt to bind a variable inside a negation.
bind_var_errors.m:031:   Variable `X' has instantiatedness `free',
bind_var_errors.m:031:   expected instantiatedness was `unique(42)'.
bind_var_errors.m:037: In clause for `bind_var_in_ite_cond(in)':
bind_var_errors.m:037:   scope error: attempt to bind a non-local variable inside the
bind_var_errors.m:037:   condition of an if-then-else.
bind_var_errors.m:037:   Variable `Y' has instantiatedness `free',
bind_var_errors.m:037:   expected instantiatedness was `unique(42)'.
bind_var_errors.m:046: In clause for `bind_var_in_lambda':
bind_var_errors.m:046:   scope error: attempt to bind a non-local variable inside
bind_var_errors.m:046:   a pred lambda goal.
bind_var_errors.m:046:   Variable `Y' has instantiatedness `free',
bind_var_errors.m:046:   expected instantiatedness was `unique(42)'.
bind_var_errors.m:056: In clause for `clobber_var_in_lambda(di)':
bind_var_errors.m:056:   in argument 1 of call to predicate `bind_var_errors:destroy/1':
bind_var_errors.m:056:   unique-mode error: the called procedure would clobber
bind_var_errors.m:056:   its argument, but variable `X' is still live.
For more information, try recompiling with `-E'.

New File: tests/invalid/bind_var_errors.m
===================================================================
:- module bind_var_errors.
:- interface.

:- import_module int.

:- pred bind_var_in_negation is semidet.

:- pred bind_var_in_ite_cond(int :: in) is semidet.

:- pred bind_var_in_lambda is semidet.

% :- pred share_var_in_lambda(T :: di) is det.

:- pred clobber_var_in_lambda(T :: di) is det.

:- implementation.

:- pragma no_inline(consume/1).
:- pred consume(T :: in) is det.
consume(_).

:- pragma no_inline(destroy/1).
:- pred destroy(T :: di) is det.
destroy(_).

:- pragma no_inline(share/1).
:- pred share(T :: in) is det.
share(_).

bind_var_in_negation :-
	\+ (X = 42),
	consume(X).

bind_var_in_ite_cond(X) :-
	(
		X = 42,
		Y = 42
	->
		true
	;
		true
	),
	consume(Y).

bind_var_in_lambda :-
	call((pred) is det :- Y = 42),
	consume(Y).

	% The compiler currently does not pass this test.  It should
	% report an error but doesn't.
% share_var_in_lambda(X) :-
% 	call((pred) is det :- share(X)),
% 	destroy(X).

clobber_var_in_lambda(X) :-
	call((pred) is det :- destroy(X)),
	destroy(X).