[m-rev.] for review: bug fix for global variables

[Mark Brown]

Hi all,

This change fixes a Heisenbug that was reported by Maria.

Cheers,
Mark.

Estimated hours taken: 20
Branches: main

When creating a new backtrackable reference that uses choicepoint ids
to avoid redundant trailing, set the initial value of the id using
MR_null_choicepoint_id() rather than MR_current_choicepoint_id().
If the current id is used, any updates that are performed before the
next choicepoint is created will not be trailed since the code for
updating the reference checks the stored id against the id that is
current at the time of the update, and only adds a trail entry if they
differ.

Normally this wouldn't be a problem because if execution ever does
backtrack to that initial choicepoint then it means it has backtracked
to a point before the reference was created, hence the reference should
no longer be live and it wouldn't matter that the reference was not
untrailed back to its initial value.  In this case the only effect of
this change is that at most one unnecessary entry may be added to the
trail.

However, if a predicate that creates such a reference is tabled, then the
reference will still exist in the table even after backtracking, and will
be produced again if that predicate is later called.  At this point the
reference will not necessarily have its correct initial value.

doc/reference_manual.texi:
	In the section on avoiding redundant trailing, advise users to
	use MR_null_choicepoint_id() if a mutable data structure is
	created by a predicate or function that is tabled.

extras/references/reference.m:
	Use MR_null_choicepoint_id() in the implementation of
	new_reference/2, and document the reason.

extras/references/tests/Mmakefile:
extras/references/tests/glob_test_2.m:
extras/references/tests/glob_test_2.exp:
	A test case that fails in grade asm_fast.gc.tr with the previous
	version of reference.m, but succeeds now.

Index: doc/reference_manual.texi
===================================================================
RCS file: /home/mercury1/repository/mercury/doc/reference_manual.texi,v
retrieving revision 1.301
diff -u -r1.301 reference_manual.texi
--- doc/reference_manual.texi	16 Dec 2004 03:23:12 -0000	1.301
+++ doc/reference_manual.texi	20 Dec 2004 17:13:11 -0000
@@ -7856,9 +7856,6 @@
 When you create a mutable data structure, you should call
 @code{MR_current_choicepoint_id()} and save the value it returns
 as a @samp{prev_choicepoint} field in your data structure.
-(If your mutable data structure
-is a C global variable, then you can use MR_null_choicepoint_id()
-for the initial value of this @samp{prev_choicepoint} field.)
 When you are about to modify your mutable data structure,
 you can then call @code{MR_current_choicepoint_id()} again and
 compare the result from that call with the value saved in
@@ -7873,7 +7870,22 @@
 @code{prev_choicepoint} field, then you can safely perform
 the update to your data structure without trailing it.
 
-For an example, see the sample module below.
+If your mutable data structure is a C global variable,
+then you can use @code{MR_null_choicepoint_id()}
+for the initial value of the @samp{prev_choicepoint} field.
+If on the other hand your mutable data structure
+is created by a predicate or function that uses tabled evaluation
+(@pxref{Tabled evaluation}),
+then you @emph{should} use @code{MR_null_choicepoint_id()}
+for the initial value of the field.
+Doing so will ensure that the data will be reset to its initial value
+if execution backtracks to a point before
+the mutable data structure was created,
+which is important because this copy of the mutable data structure
+will be tabled and will therefore be produced again
+if later execution attempts to create another instance of it.
+
+For an example of avoiding redundant trailing, see the sample module below.
 
 Note that there is a cost to this --- you have to include
 an extra field in your data structure for each part of
Index: extras/references/reference.m
===================================================================
RCS file: /home/mercury1/repository/mercury/extras/references/reference.m,v
retrieving revision 1.5
diff -u -r1.5 reference.m
--- extras/references/reference.m	10 Jan 2003 05:49:22 -0000	1.5
+++ extras/references/reference.m	20 Dec 2004 17:13:11 -0000
@@ -63,7 +63,13 @@
 	MR_incr_hp(Ref, (sizeof(ME_Reference) + sizeof(MR_Word) - 1) / 
 			sizeof(MR_Word));
 	((ME_Reference *) Ref)->value = (void *) X;
-	((ME_Reference *) Ref)->id = MR_current_choicepoint_id();
+		/*
+		** Use MR_null_choicepoint_id here instead of
+		** MR_current_choicepoint_id, in case this is called from
+		** a tabled pred/func -- even if it isn't, this will only
+		** result in one additional (redundant) entry on the trail.
+		*/
+	((ME_Reference *) Ref)->id = MR_null_choicepoint_id();
 ").
 
 :- pragma inline(value/2).
Index: extras/references/tests/Mmakefile
===================================================================
RCS file: /home/mercury1/repository/mercury/extras/references/tests/Mmakefile,v
retrieving revision 1.5
diff -u -r1.5 Mmakefile
--- extras/references/tests/Mmakefile	12 Aug 2003 07:01:35 -0000	1.5
+++ extras/references/tests/Mmakefile	20 Dec 2004 17:13:11 -0000
@@ -26,7 +26,7 @@
 
 #-----------------------------------------------------------------------------#
 
-PROGS	=	ref_test glob_test
+PROGS	=	ref_test glob_test glob_test_2
 
 DEPENDS =	$(PROGS:%=%.depend)
 CS	=	$(PROGS:%=%.c)
Index: extras/references/tests/glob_test_2.exp
===================================================================
RCS file: extras/references/tests/glob_test_2.exp
diff -N extras/references/tests/glob_test_2.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ extras/references/tests/glob_test_2.exp	20 Dec 2004 17:13:11 -0000
@@ -0,0 +1 @@
+1
Index: extras/references/tests/glob_test_2.m
===================================================================
RCS file: extras/references/tests/glob_test_2.m
diff -N extras/references/tests/glob_test_2.m
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ extras/references/tests/glob_test_2.m	20 Dec 2004 17:13:11 -0000
@@ -0,0 +1,34 @@
+:- module glob_test_2.
+:- interface.
+:- import_module io.
+
+:- pred main(io::di, io::uo) is det.
+
+:- implementation.
+:- import_module std_util, reference.
+
+:- pragma c_header_code("
+	#include ""c_reference.h""
+").
+
+:- pragma promise_pure(main/2).
+
+main -->
+	(
+		{ impure update(trailed_global, 2) },
+		{ semidet_fail }
+	->
+		write_string("I didn't expect that to succeed!\n")
+	;
+		{ semipure value(trailed_global, Value) },
+		write_int(Value),
+		nl
+	).
+
+:- func trailed_global = reference(int).
+:- pragma memo(trailed_global/0).
+:- pragma promise_pure(trailed_global/0).
+
+trailed_global = Ref :-
+	impure new_reference(1, Ref).
+
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------