[m-rev.] Re: diff: asm_fast.gc.par bug fix & more cc_nondet => cc_multi

Fergus Henderson fjh at cs.mu.OZ.AU
Tue Jan 21 00:14:45 AEDT 2003


Sorry, I was a bit quick on the trigger-finger there -- it helps to at
least compile the code before posting it for review!  Here's an updated
version which actually compiles.  I've enclosed both the new full diff
and the relative diff against the version that I posted previously.

*********
FULL DIFF
*********

Estimated hours taken: 9
Branches: main

Finish off Peter Ross's partially-completed set of changes to
use `cc_multi' instead of `cc_nondet' for various predicates in
deconstruct.m and std_util.m.  Also, fix a bug in his changes that
broke `deconstruct.arg_cc' in the asm_fast.gc.par grade.

library/std_util.m:
	Changed named_argument_cc from `cc_nondet' to `cc_multi'.

library/deconstruct.m:
	Delete the cc_nondet mode of `named_arg',
	and replace it with a new cc_multi predicate `named_arg_cc'.

	Also, fix a bug where univ_arg_idcc was declared
	`will_not_call_mercury' but was nevertheless calling Mercury code.

NEWS:
	Document the interface changes mentioned above.

	Also, reorganize the description of all the changes relating
	to incorrect use of `cc_nondet' into a single section.

Workspace: /home/ceres/fjh/mercury
Index: NEWS
===================================================================
RCS file: /home/mercury1/repository/mercury/NEWS,v
retrieving revision 1.293
diff -u -d -r1.293 NEWS
--- NEWS	17 Jan 2003 05:56:46 -0000	1.293
+++ NEWS	20 Jan 2003 11:52:41 -0000
@@ -49,17 +49,18 @@
   `is_nan_or_inf/1' to float.m.  These predicates are for use only on
   systems which support IEEE floating point arithmetic.
 
-* The determinisms of the following predicates in the `std_util'
-  module have been changed from cc_nondet to cc_multi: arg_cc/3,
-  argument_cc/3 and limited_deconstruct_cc/3 (formerly
-  limited_deconstruct_cc/5).  The success or failure of these
-  predicates is now encoded in a maybe type.
+* We've fixed some problems with the use of `cc_nondet'.
 
-* The incorrect cc_nondet modes of the following predicates in
-  the `deconstruct' module have been removed: arg/4, limited_deconstruct/6.
-  cc_multi version of the predicates have been introduced where the
-  success or failure of the predicate has been encoded into a maybe
-  type.
+  The incorrect cc_nondet modes of the following predicates have been removed:
+	deconstruct.arg/4
+	deconstruct.named_arg/4
+	deconstruct.limited_deconstruct/6
+	std_util.arg_cc/3
+	std_util.argument_cc/3
+	std_util.named_argument_cc/3
+	std_util.limited_deconstruct_cc/5
+  These have been replaced by cc_multi versions in which success or failure
+  is indicated by returning a maybe type.
 
 Changes to the extras distribution:
 
Index: library/deconstruct.m
===================================================================
RCS file: /home/mercury1/repository/mercury/library/deconstruct.m,v
retrieving revision 1.22
diff -u -d -r1.22 deconstruct.m
--- library/deconstruct.m	16 Jan 2003 16:21:28 -0000	1.22
+++ library/deconstruct.m	20 Jan 2003 12:55:48 -0000
@@ -128,7 +128,9 @@
 :- mode arg(in, in(canonicalize), in, out) is semidet.
 :- mode arg(in, in(canonicalize_or_do_not_allow), in, out) is semidet.
 
-	% See the documentation of std_util__arg_cc
+	% arg_cc/3 is similar to arg/4, except that it
+	% handles arguments with non-canonical types.
+	% See the documentation of std_util__arg_cc.
 :- pred arg_cc(T, int, std_util__maybe_arg).
 :- mode arg_cc(in, in, out) is cc_multi.
 
@@ -141,8 +143,12 @@
 :- some [ArgT] pred named_arg(T, noncanon_handling, string, ArgT).
 :- mode named_arg(in, in(do_not_allow), in, out) is semidet.
 :- mode named_arg(in, in(canonicalize), in, out) is semidet.
-:- mode named_arg(in, in(include_details_cc), in, out) is cc_nondet.
-:- mode named_arg(in, in, in, out) is cc_nondet.
+:- mode named_arg(in, in(canonicalize_or_do_not_allow), in, out) is semidet.
+
+	% named_arg_cc/3 is similar to named_arg/4, except that it
+	% handles arguments with non-canonical types.
+:- pred named_arg_cc(T, string, std_util__maybe_arg) is cc_multi.
+:- mode named_arg_cc(in, in, out) is cc_multi.
 
 	% det_arg(Data, NonCanon, Index, Argument)
 	%
@@ -293,10 +299,10 @@
 	Argument = univ_value(Univ).
 
 arg_cc(Term, Index, MaybeArg) :-
-	univ_arg_idcc(Term, Index, MaybeUniv),
-	( MaybeUniv = yes(Univ),
+	univ_arg_idcc(Term, Index, dummy_univ, Univ, Success),
+	( Success \= 0 ->
 		MaybeArg = 'new arg'(univ_value(Univ))
-	; MaybeUniv = no,
+	;
 		MaybeArg = std_util__no_arg
 	).
 
@@ -309,31 +315,47 @@
 		univ_named_arg_can(Term, Name, Univ)
 	;
 		NonCanon = include_details_cc,
-		univ_named_arg_idcc(Term, Name, Univ)
+		error("deconstruct__named_arg called with include_details_cc")
 	),
 	Argument = univ_value(Univ).
 
+named_arg_cc(Term, Name, MaybeArg) :-
+	univ_named_arg_idcc(Term, Name, dummy_univ, Univ, Success),
+	( Success \= 0 ->
+		MaybeArg = 'new arg'(univ_value(Univ))
+	;
+		MaybeArg = std_util__no_arg
+	).
+
+	% This is a dummy value of type `univ'.
+	% It is used only to ensure that the C interface procedure
+	% univ_named_arg_idcc doesn't return an uninitialized
+	% (or otherwise bogus) univ value.
+:- func dummy_univ = univ.
+dummy_univ = univ(0).
+
 det_arg(Term, NonCanon, Index, Argument) :-
 	( NonCanon = do_not_allow,
-		( univ_arg_dna(Term, Index, Univ) ->
-			MaybeArg = yes(Univ)
+		( univ_arg_dna(Term, Index, Univ0) ->
+			Univ = Univ0
 		;
-			MaybeArg = no
+			error("det_arg: argument number out of range")
 		)
 	; NonCanon = canonicalize,
-		( univ_arg_can(Term, Index, Univ) ->
-			MaybeArg = yes(Univ)
+		( univ_arg_can(Term, Index, Univ0) ->
+			Univ = Univ0
 		;
-			MaybeArg = no
+			error("det_arg: argument number out of range")
 		)
 	; NonCanon = include_details_cc,
-		univ_arg_idcc(Term, Index, MaybeArg)
+		univ_arg_idcc(Term, Index, dummy_univ, Univ0, Success),
+		( Success \= 0 ->
+			Univ = Univ0
+		;
+			error("det_arg: argument number out of range")
+		)
 	),
-	( MaybeArg = yes(UnivArg),
-		Argument = univ_value(UnivArg)
-	; MaybeArg = no,
-		error("det_arg: argument number out of range")
-	).
+	Argument = univ_value(Univ).
 
 det_named_arg(Term, NonCanon, Name, Argument) :-
 	(
@@ -345,7 +367,13 @@
 			univ_named_arg_can(Term, Name, Univ)
 		;
 			NonCanon = include_details_cc,
-			univ_named_arg_idcc(Term, Name, Univ)
+			univ_named_arg_idcc(Term, Name, dummy_univ, Univ0,
+				Success),
+			( Success \= 0 ->
+				Univ = Univ0
+			;
+				error("det_named_arg: no argument with that name")
+			)
 		)
 	->
 		Argument = univ_value(Univ)
@@ -458,15 +486,31 @@
 
 % XXX These predicates return univs instead of existentially typed arguments
 % in order to work around the typechecking bug reported on 30 Jan, 2002
-% to the mercury-bugs mailing list, and which has sourceforge bug id 512581.
+% to the mercury-bugs mailing list, and which has sourceforge bug id 512581:
+% currently we don't support implementations in multiple languages
+% for procedures with existentially typed arguments.
 
 :- pred univ_arg_dna(T::in, int::in, univ::out) is semidet.
 :- pred univ_arg_can(T::in, int::in, univ::out) is semidet.
-:- pred univ_arg_idcc(T::in, int::in, maybe(univ)::out) is cc_multi.
+
+	% univ_arg_idcc(Term, N, DummyUniv, Argument, Success):
+	%	Attempt to extract the Nth field of (the current
+	%	representation of) Term.  If there is such a field,
+	%	return Success=1 and return the field in Argument.
+	%	If there is not, return Success=0 and Argument=DummyUniv.
+:- pred univ_arg_idcc(T::in, int::in, univ::in, univ::out, int::out)
+	is cc_multi.
 
 :- pred univ_named_arg_dna(T::in, string::in, univ::out) is semidet.
 :- pred univ_named_arg_can(T::in, string::in, univ::out) is semidet.
-:- pred univ_named_arg_idcc(T::in, string::in, univ::out) is cc_nondet.
+
+	% univ_named_arg_idcc(Term, Name, DummyUniv, Univ, Success):
+	%	Attempt to extract the field of (the current representation of)
+	%	Term specified by Name.  If there is such a field,
+	%	return Success=1 and return the field in Univ.
+	%	If there is not, return Success=0 and Univ=DummyUniv.
+:- pred univ_named_arg_idcc(T::in, string::in, univ::in, univ::out, int::out)
+	is cc_multi.
 
 :- pragma foreign_proc("C",
 	univ_arg_dna(Term::in, Index::in, Argument::out),
@@ -511,11 +555,10 @@
 }").
 
 :- pragma foreign_proc("C",
-	univ_arg_idcc(Term::in, Index::in, MaybeArg::out),
+	univ_arg_idcc(Term::in, Index::in, DummyUniv::in, Argument::out,
+		Success::out),
 	[will_not_call_mercury, thread_safe, promise_pure],
 "{
-	MR_Word Argument;
-
 	#define	TYPEINFO_ARG		TypeInfo_for_T
 	#define	TERM_ARG		Term
 	#define	SELECTOR_ARG		Index
@@ -531,12 +574,10 @@
 	#undef	NONCANON
 
 	if (success) {
-		MaybeArg = ML_construct_maybe_yes((MR_Word)
-				&MR_TYPE_CTOR_INFO_NAME(std_util, univ, 0),
-				Argument);
+		Success = 1;
 	} else {
-		MaybeArg = ML_construct_maybe_no((MR_Word)
-				&MR_TYPE_CTOR_INFO_NAME(std_util, univ, 0));
+		Success = 0;
+		Argument = DummyUniv;
 	}
 }").
 
@@ -587,7 +628,8 @@
 }").
 
 :- pragma foreign_proc("C",
-	univ_named_arg_idcc(Term::in, Name::in, Argument::out),
+	univ_named_arg_idcc(Term::in, Name::in, DummyUniv::in,
+	Argument::out, Success::out),
 	[will_not_call_mercury, thread_safe, promise_pure],
 "{
 #define	TYPEINFO_ARG		TypeInfo_for_T
@@ -597,7 +639,6 @@
 #define	SELECTED_TYPE_INFO	TypeInfo_for_ArgT
 #define	NONCANON		MR_NONCANON_CC
 #define	SELECT_BY_NAME
-#define	SAVE_SUCCESS
 #include ""mercury_ml_arg_body.h""
 #undef	TYPEINFO_ARG
 #undef	TERM_ARG
@@ -606,9 +647,21 @@
 #undef	SELECTED_TYPE_INFO
 #undef	NONCANON
 #undef	SELECT_BY_NAME
-#undef	SAVE_SUCCESS
+	
+	if (success) {
+		Success = 1;
+	} else {
+		Success = 0;
+		Argument = DummyUniv;
+	}
+
 }").
 
+% XXX These Mercury implementations are all inefficient,
+%     since they unnecessarily construct the list of univs
+%     for all the arguments, rather than just constructing
+%     one univ for the argument selected.
+
 univ_arg_dna(Term::in, Index::in, Arg::out) :-
 	rtti_implementation__deconstruct(Term,
 			do_not_allow, _Functor, _Arity, Arguments),
@@ -617,13 +670,16 @@
 	rtti_implementation__deconstruct(Term,
 			canonicalize, _Functor, _Arity, Arguments),
 	list__index0(Arguments, Index, Arg).
-univ_arg_idcc(Term::in, Index::in, MaybeArg::out) :-
+univ_arg_idcc(Term::in, Index::in, DummyUniv::in, Argument::out,
+		Success::out) :-
 	rtti_implementation__deconstruct(Term,
 			include_details_cc, _Functor, _Arity, Arguments),
 	( list__index0(Arguments, Index, Arg) ->
-		MaybeArg = yes(Arg)
+		Argument = Arg,
+		Success = 1
 	;
-		MaybeArg = no
+		Argument = DummyUniv,
+		Success = 0
 	).
 
 univ_named_arg_dna(_Term::in, _Name::in, _Arg::out) :-
@@ -634,7 +690,8 @@
 	% This version is only used for back-ends for which there is no
 	% matching foreign_proc version.
 	private_builtin__sorry("deconstruct__univ_named_arg_can/3").
-univ_named_arg_idcc(_Term::in, _Name::in, _Arg::out) :-
+univ_named_arg_idcc(_Term::in, _Name::in, _DummyUniv::in, _Argument::out,
+		_Success::out) :-
 	% This version is only used for back-ends for which there is no
 	% matching foreign_proc version.
 	private_builtin__sorry("deconstruct__univ_named_arg_idcc/3").
Index: library/std_util.m
===================================================================
RCS file: /home/mercury1/repository/mercury/library/std_util.m,v
retrieving revision 1.278
diff -u -d -r1.278 std_util.m
--- library/std_util.m	2 Jan 2003 06:53:58 -0000	1.278
+++ library/std_util.m	20 Jan 2003 12:51:29 -0000
@@ -652,7 +652,7 @@
 	% of a non-canonical type.
 	%
 :- func named_argument(T::in, string::in) = (univ::out) is semidet.
-:- pred named_argument_cc(T::in, string::in, univ::out) is cc_nondet.
+:- pred named_argument_cc(T::in, string::in, maybe(univ)::out) is cc_multi.
 
 	% det_arg(Data, ArgumentIndex) = Argument
 	% det_argument(Data, ArgumentIndex) = ArgumentUniv
@@ -798,16 +798,6 @@
 		Y = no
 	).
 
-	% Utility predicates which are useful for constructing values
-	% of the maybe(T) type from foreign code.
-:- func construct_yes(T) = maybe(T).
-:- pragma export(construct_yes(in) = out, "ML_construct_maybe_yes").
-construct_yes(T) = yes(T).
-
-:- func construct_no = maybe(T).
-:- pragma export(construct_no = out, "ML_construct_maybe_no").
-construct_no = no.
-
 %-----------------------------------------------------------------------------%
 
 /*
@@ -1644,10 +1634,16 @@
 	deconstruct__named_arg(Term, canonicalize, Name, Argument),
 	type_to_univ(Argument, ArgumentUniv).
 
-named_argument_cc(Term, Name, ArgumentUniv) :-
-	deconstruct__named_arg(Term, include_details_cc,
-		Name, Argument),
-	type_to_univ(Argument, ArgumentUniv).
+named_argument_cc(Term, Name, MaybeArgumentUniv) :-
+	deconstruct__named_arg_cc(Term, Name, MaybeArgument),
+	(
+		MaybeArgument = arg(Argument),
+		type_to_univ(Argument, ArgumentUniv),
+		MaybeArgumentUniv = yes(ArgumentUniv)
+	;
+		MaybeArgument = no_arg,
+		MaybeArgumentUniv = no
+	).
 
 deconstruct(Term, Functor, Arity, Arguments) :-
 	deconstruct__deconstruct(Term, canonicalize,

*************
RELATIVE DIFF
*************

--- /tmp/fjh/mercury/test_cc_nondet_fix/mercury/library/std_util.m	Mon Jan 20 23:31:26 2003
+++ ./std_util.m	Mon Jan 20 23:51:29 2003
@@ -1634,10 +1634,16 @@
 	deconstruct__named_arg(Term, canonicalize, Name, Argument),
 	type_to_univ(Argument, ArgumentUniv).
 
-named_argument_cc(Term, Name, ArgumentUniv) :-
-	deconstruct__named_arg(Term, include_details_cc,
-		Name, Argument),
-	type_to_univ(Argument, ArgumentUniv).
+named_argument_cc(Term, Name, MaybeArgumentUniv) :-
+	deconstruct__named_arg_cc(Term, Name, MaybeArgument),
+	(
+		MaybeArgument = arg(Argument),
+		type_to_univ(Argument, ArgumentUniv),
+		MaybeArgumentUniv = yes(ArgumentUniv)
+	;
+		MaybeArgument = no_arg,
+		MaybeArgumentUniv = no
+	).
 
 deconstruct(Term, Functor, Arity, Arguments) :-
 	deconstruct__deconstruct(Term, canonicalize,
--- /tmp/fjh/mercury/test_cc_nondet_fix/mercury/library/deconstruct.m	Mon Jan 20 23:31:26 2003
+++ ./deconstruct.m	Mon Jan 20 23:55:48 2003
@@ -299,10 +299,10 @@
 	Argument = univ_value(Univ).
 
 arg_cc(Term, Index, MaybeArg) :-
-	univ_arg_idcc(Term, Index, MaybeUniv),
-	( MaybeUniv = yes(Univ),
+	univ_arg_idcc(Term, Index, dummy_univ, Univ, Success),
+	( Success \= 0 ->
 		MaybeArg = 'new arg'(univ_value(Univ))
-	; MaybeUniv = no,
+	;
 		MaybeArg = std_util__no_arg
 	).
 
@@ -319,7 +319,7 @@
 	),
 	Argument = univ_value(Univ).
 
-named_arg_cc(Term, Name, Argument) :-
+named_arg_cc(Term, Name, MaybeArg) :-
 	univ_named_arg_idcc(Term, Name, dummy_univ, Univ, Success),
 	( Success \= 0 ->
 		MaybeArg = 'new arg'(univ_value(Univ))
@@ -336,25 +336,26 @@
 
 det_arg(Term, NonCanon, Index, Argument) :-
 	( NonCanon = do_not_allow,
-		( univ_arg_dna(Term, Index, Univ) ->
-			Argument = univ_value(UnivArg)
-		; MaybeArg = no,
+		( univ_arg_dna(Term, Index, Univ0) ->
+			Univ = Univ0
+		;
 			error("det_arg: argument number out of range")
 		)
 	; NonCanon = canonicalize,
-		( univ_arg_can(Term, Index, Univ) ->
-			Argument = univ_value(Univ)
-		; MaybeArg = no,
+		( univ_arg_can(Term, Index, Univ0) ->
+			Univ = Univ0
+		;
 			error("det_arg: argument number out of range")
 		)
 	; NonCanon = include_details_cc,
-		univ_arg_idcc(Term, Index, dummy_univ, UnivArg, Success),
+		univ_arg_idcc(Term, Index, dummy_univ, Univ0, Success),
 		( Success \= 0 ->
-			Argument = univ_value(UnivArg)
+			Univ = Univ0
 		;
 			error("det_arg: argument number out of range")
 		)
-	).
+	),
+	Argument = univ_value(Univ).
 
 det_named_arg(Term, NonCanon, Name, Argument) :-
 	(
@@ -669,13 +670,16 @@
 	rtti_implementation__deconstruct(Term,
 			canonicalize, _Functor, _Arity, Arguments),
 	list__index0(Arguments, Index, Arg).
-univ_arg_idcc(Term::in, Index::in, MaybeArg::out) :-
+univ_arg_idcc(Term::in, Index::in, DummyUniv::in, Argument::out,
+		Success::out) :-
 	rtti_implementation__deconstruct(Term,
 			include_details_cc, _Functor, _Arity, Arguments),
 	( list__index0(Arguments, Index, Arg) ->
-		MaybeArg = yes(Arg)
+		Argument = Arg,
+		Success = 1
 	;
-		MaybeArg = no
+		Argument = DummyUniv,
+		Success = 0
 	).
 
 univ_named_arg_dna(_Term::in, _Name::in, _Arg::out) :-
@@ -686,7 +690,8 @@
 	% This version is only used for back-ends for which there is no
 	% matching foreign_proc version.
 	private_builtin__sorry("deconstruct__univ_named_arg_can/3").
-univ_named_arg_idcc(_Term::in, _Name::in, _Arg::out) :-
+univ_named_arg_idcc(_Term::in, _Name::in, _DummyUniv::in, _Argument::out,
+		_Success::out) :-
 	% This version is only used for back-ends for which there is no
 	% matching foreign_proc version.
 	private_builtin__sorry("deconstruct__univ_named_arg_idcc/3").

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------



More information about the reviews mailing list