[m-rev.] diff: fix bugs in smart recompilation

[Simon Taylor]

On 24-Jul-2001, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> On 24-Jul-2001, Simon Taylor <stayl at cs.mu.OZ.AU> wrote:
> > 
> > +:- func item_is_unchanged(item, item) = bool.
> > +
> > +	% We don't need to compare the varsets. What matters is that
> > +	% the variable numbers in the arguments and body are the same,
> > +	% the names are irrelevant.
> 
> Hmm, that looks wrong to me.  Maybe you're not aware of the following sentence
> from the Mercury language reference manual:
> 
>  | The scope of type variables in a predicate or function's type
>  | declaration extends over any explicit type qualifications in the
>  | clauses for that predicate or function, so explicit type qualifications
>  | can refer to those type variables.
> 
> So if, for example, you write
> 
> 	:- module example.
> 	:- interface.
> 	:- import_module io.
> 
> 	:- pred main(io__state::di, io__state::uo) is det.
> 
> 	:- implementation.
> 
> 	main --> foo(0, "").
> 
> 	:- pred foo(T1, T2, io__state, io__state).
> 	:- mode foo(in, in, di, uo) is det.
> 
> 	foo(_, _) -->
> 		io__read(R `with_type` io__read_result(T1)),
> 		io__print(R),
> 		io__nl.
> 
> and then you change the pred declaration for `foo' to
> 
> 	:- pred foo(T2, T1, io__state, io__state).
> 
> then the resulting program will behave differently.

But to do that you have to modify the `.m' file, which will cause
the module to be recompiled.

The only other place where the names of variables matters is in
`:- pragma type_spec' declarations. My change did break the
handling of those.

Simon.

Estimated hours taken: 1
Branches: main

Fix a bug in my last change.

compiler/recompilation_version.m:
	My last change assumed that the names of variables do not
	matter. That is incorrect in the case of explicit type
	qualification and `:- pragma type_spec' declarations.
	recompilation_version.m only deals with items in interface
	files, so explicit type qualification does not need to
	be considered. For `:- pragma type_spec' declarations to
	work we need to consider a predicate or function declaration
	changed if the names of the type variables in the predicate
	declaration change.

tests/recompilation/TESTS:
tests/recompilation/type_spec_rename_var_r*:
	Test case.

Index: compiler/recompilation_version.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/recompilation_version.m,v
retrieving revision 1.3
diff -u -u -r1.3 recompilation_version.m
--- compiler/recompilation_version.m	2001/07/24 10:47:16	1.3
+++ compiler/recompilation_version.m	2001/07/24 16:52:44
@@ -499,11 +499,20 @@
 	yes = item_is_unchanged(Item1, Item2),
 	items_are_unchanged(Items1, Items2).
 
-:- func item_is_unchanged(item, item) = bool.
-
-	% We don't need to compare the varsets. What matters is that
-	% the variable numbers in the arguments and body are the same,
-	% the names are irrelevant.
+	% In most places here, we don't need to compare the varsets.
+	% What matters is that the variable numbers in the arguments
+	% and body are the same, the names are usually irrelevant.
+	%
+	% The only places where the names of variables affect the
+	% compilation of the program are in explicit type qualifications
+	% and `:- pragma type_spec' declarations. Explicit type
+	% qualifications do not need to be considered here. This module
+	% only deals with items in interface files (we don't yet write type
+	% qualifications to `.opt' files). Variables in type qualifications
+	% are only matched with the head type variables of the predicate
+	% by make_hlds.m. For `:- pragma type_spec' declarations to work
+	% we need to consider a predicate or function declaration to be
+	% changed if the names of any of the type variables are changed.
 	%
 	% It's important not to compare the varsets for type and instance
 	% declarations because the declarations we get here may be abstract
@@ -517,6 +526,8 @@
 	% those from the body, so that the variable numbers in the head of
 	% the declaration match those from an abstract declaration read
 	% from an interface file.
+:- func item_is_unchanged(item, item) = bool.
+
 item_is_unchanged(type_defn(_VarSet, Name, Args, Defn, Cond), Item2) =
 		( Item2 = type_defn(_, Name, Args, Defn, Cond) -> yes ; no ).
 item_is_unchanged(mode_defn(_VarSet, Name, Args, Defn, Cond), Item2) =
@@ -541,6 +552,10 @@
 item_is_unchanged(assertion(Goal, _VarSet), Item2) =
 		( Item2 = assertion(Goal, _) -> yes ; no ).
 
+	% We do need to compare the varset in `:- pragma type_spec'
+	% declarations because the names of the variables are used
+	% to find the corresponding variables in the predicate or
+	% function type declaration.
 item_is_unchanged(pragma(PragmaType), Item2) =
 		( Item2 = pragma(PragmaType) -> yes ; no ).
 item_is_unchanged(nothing(A), Item2) =
@@ -610,7 +625,7 @@
 		Constraints1, TVarSet2, ExistQVars2,
 		TypesAndModes2, Constraints2) :-
 
-	varset__merge_subst(TVarSet1, TVarSet2, _, Subst),
+	varset__merge_subst(TVarSet1, TVarSet2, TVarSet, Subst),
 
 	GetArgTypes =
 		(func(TypeAndMode0) = Type :-
@@ -633,6 +648,33 @@
 	%
 	type_list_subsumes(SubstTypes2, Types1, Types2ToTypes1Subst),
 	type_list_subsumes(Types1, SubstTypes2, _),
+
+	%
+	% Check that the corresponding variables have the same names.
+	% This is necessary because `:- pragma type_spec' declarations
+	% depend on the names of the variables, so for example if two
+	% variable names are swapped, the same `:- pragma type_spec'
+	% declaration will cause a different specialized version to be
+	% created.
+	%
+	( all [VarInItem1, VarInItem2]
+	    (
+		map__member(Types2ToTypes1Subst, VarInItem2, SubstTerm),
+		(
+			SubstTerm = term__variable(VarInItem1)
+		;
+			% The reverse subsumption test above should
+			% ensure that the substitutions are all var->var.
+			SubstTerm = term__functor(_, _, _),
+			error("pred_or_func_type_matches: invalid subst")
+		)
+	    )
+	=>
+	    (
+		varset__lookup_name(TVarSet, VarInItem1, VarName),
+		varset__lookup_name(TVarSet, VarInItem2, VarName)
+	    )
+	),
 
 	%
 	% Check that the existentially quantified variables are equivalent.
Index: tests/recompilation/TESTS
===================================================================
RCS file: /home/mercury1/repository/tests/recompilation/TESTS,v
retrieving revision 1.3
diff -u -u -r1.3 TESTS
--- tests/recompilation/TESTS	2001/07/24 10:47:21	1.3
+++ tests/recompilation/TESTS	2001/07/24 15:18:14
@@ -20,7 +20,8 @@
 	pragma_type_spec_r \
 	pred_ctor_ambiguity_r \
 	pred_overloading_r \
-	typeclass_method_pragma_r"
+	typeclass_method_pragma_r \
+	type_spec_rename_var_r"
 
 # Parallel mmake with nested sub-modules is broken.
 # The commands to create `.c' files from the `.m' file containing the
Index: tests/recompilation/type_spec_rename_var_r.err_exp.2
===================================================================
RCS file: type_spec_rename_var_r.err_exp.2
diff -N type_spec_rename_var_r.err_exp.2
--- /dev/null	Mon Apr 16 11:57:05 2001
+++ type_spec_rename_var_r.err_exp.2	Wed Jul 25 01:21:34 2001
@@ -0,0 +1,2 @@
+Recompiling module `type_spec_rename_var_r':
+  predicate `type_spec_rename_var_r_2:p/3' was modified.
Index: tests/recompilation/type_spec_rename_var_r.exp.1
===================================================================
RCS file: type_spec_rename_var_r.exp.1
diff -N type_spec_rename_var_r.exp.1
--- /dev/null	Mon Apr 16 11:57:05 2001
+++ type_spec_rename_var_r.exp.1	Wed Jul 25 01:21:30 2001
@@ -0,0 +1 @@
+1 - 2.00000000000000
Index: tests/recompilation/type_spec_rename_var_r.exp.2
===================================================================
RCS file: type_spec_rename_var_r.exp.2
diff -N type_spec_rename_var_r.exp.2
--- /dev/null	Mon Apr 16 11:57:05 2001
+++ type_spec_rename_var_r.exp.2	Wed Jul 25 01:21:34 2001
@@ -0,0 +1 @@
+1 - 2.00000000000000
Index: tests/recompilation/type_spec_rename_var_r.m.1
===================================================================
RCS file: type_spec_rename_var_r.m.1
diff -N type_spec_rename_var_r.m.1
--- /dev/null	Mon Apr 16 11:57:05 2001
+++ type_spec_rename_var_r.m.1	Wed Jul 25 01:19:48 2001
@@ -0,0 +1,16 @@
+:- module type_spec_rename_var_r.
+
+:- interface.
+
+:- import_module io.
+
+:- pred main(io__state::di, io__state::uo) is det.
+
+:- implementation.
+
+:- import_module type_spec_rename_var_r_2.
+
+main -->
+	{ p(1, 2.0, Result) },
+	io__write(Result),
+	io__nl.
Index: tests/recompilation/type_spec_rename_var_r_2.err_exp.2
===================================================================
RCS file: type_spec_rename_var_r_2.err_exp.2
diff -N type_spec_rename_var_r_2.err_exp.2
--- /dev/null	Mon Apr 16 11:57:05 2001
+++ type_spec_rename_var_r_2.err_exp.2	Wed Jul 25 01:21:34 2001
@@ -0,0 +1,2 @@
+Recompiling module `type_spec_rename_var_r_2':
+  file `type_spec_rename_var_r_2.m' has changed.
Index: tests/recompilation/type_spec_rename_var_r_2.m.1
===================================================================
RCS file: type_spec_rename_var_r_2.m.1
diff -N type_spec_rename_var_r_2.m.1
--- /dev/null	Mon Apr 16 11:57:05 2001
+++ type_spec_rename_var_r_2.m.1	Wed Jul 25 01:21:15 2001
@@ -0,0 +1,12 @@
+:- module type_spec_rename_var_r_2.
+
+:- interface.
+
+:- import_module std_util.
+
+:- pred p(T::in, U::in, pair(T, U)::out) is det.
+:- pragma type_spec(p/3, T = int).
+
+:- implementation.
+
+p(T, U, T - U).
Index: tests/recompilation/type_spec_rename_var_r_2.m.2
===================================================================
RCS file: type_spec_rename_var_r_2.m.2
diff -N type_spec_rename_var_r_2.m.2
--- /dev/null	Mon Apr 16 11:57:05 2001
+++ type_spec_rename_var_r_2.m.2	Wed Jul 25 01:21:23 2001
@@ -0,0 +1,12 @@
+:- module type_spec_rename_var_r_2.
+
+:- interface.
+
+:- import_module std_util.
+
+:- pred p(U::in, T::in, pair(U, T)::out) is det.
+:- pragma type_spec(p/3, T = int).
+
+:- implementation.
+
+p(T, U, T - U).
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------