[m-rev.] for review: comparison of preds/funcs

[Mark Brown]

On 24-Apr-2002, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> On 22-Apr-2002, Mark Brown <dougl at cs.mu.OZ.AU> wrote:
> > +#define	start_label		compare_rep_start
> > +#define	call_user_code_label	call_compare_rep_in_proc
> > +#define	type_stat_struct	MR_type_stat_mer_compare
> > +#define	attempt_msg		"attempt to compare representation "
> > +#define	select_compare_code
> > +#define include_compare_rep_code
> > +#define	entry_point_is_mercury
> 
> The indentation here is inconsistent.

Fixed.

> 
> You defined type_stat_struct to be the same as for the ordinary compare/3,
> both here and in the C version, which means that the statistics for compare/3
> and compare_representation/3 will be conflated.

I'm not sure what to do about this one.  Should I:
	- add two new MR_TypeStat structs for the new predicate, and fill
	  those in instead;
	- not record any stats at all for the new predicate;
	- leave them conflated, but add a comment to the documentation of
	  MR_TYPE_CTOR_STATS that says that the stats for compare also
	  include calls to compare_representation;
	- do something else
?

> 
> > +#include "mercury_unify_compare_body.h"
> > +
> > +#undef  DECLARE_LOCALS
> > +#undef  initialize
> > +#undef  return_answer
> > +#undef  start_label
> > +#undef	call_user_code_label
> > +#undef  type_stat_struct
> > +#undef  attempt_msg
> > +#undef	select_compare_code
> > +#undef	include_compare_rep_code
> > +#undef	entry_point_is_mercury
> 
> The indentation here is inconsistent.

This inconsistency was cut-n-pasted from existing code.  I've fixed all
these places now.

> > +	x_layout = x->MR_closure_layout;
> > +	y_layout = y->MR_closure_layout;
> > +
> > +	x_proc_id = &x_layout->MR_closure_id->MR_closure_proc_id;
> > +	y_proc_id = &y_layout->MR_closure_id->MR_closure_proc_id;
> > +	if (x_proc_id  < y_proc_id) {
> > +		return MR_COMPARE_LESS;
> > +	} else if (x_proc_id > y_proc_id) {
> > +		return MR_COMPARE_GREATER;
> > +	}
> 
> You should document that this works as long as the MR_closure_proc_ids
> cannot be relocated, which is now guaranteed, but may not be true in the
> future.
> 
> I am not sure whether the equality of the *contents* of MR_closure_proc_id
> guarantees equality of their addresses.

I am not sure of this either, so it is probably better to be conservative
and check the contents if the addresses aren't equal.  This won't be too
much more expensive, so I've taken this approach now.

Come to think of it, we shouldn't be insisting that the modes of the two
procedures are the same.  The operational semantics of compare_representation
are that we should compare only the name, arity and arguments of the closure.
So comparing proc_ids is too strong.  I've improved the documentation of
the new predicate to make this clearer.

Also, by comparing the contents of the proc_ids, we make the operational
semantics more predictable, which means that we *can* simply output the
result in the test cases.  I've made this change to the test cases.

> > +	(
> > +		{ Res = (=) }
> > +	->
> > +		[]
> > +	;
> > +		io__write_string("maybe not ")
> > +	),
> 
> Why the maybe?

Because two non-canonical terms could be equal, but still give a result
other than (=).

Below is the relative diff.  Only the change to one of the test cases is
shown, because interdiff got confused by the others.  The changes made to
the other cases are analogous to this one.  I'm awaiting your advice on
type_stat_struct before committing.

Cheers,
Mark.

diff -u library/std_util.m library/std_util.m
--- library/std_util.m
+++ library/std_util.m
@@ -724,8 +724,17 @@
 	% compare_representation(Result, X, Y)
 	%
 	% compare_representation is similar to the builtin predicate
-	% compare/3, except that it attempts to compare non-canonical
-	% terms by comparing their representations.
+	% compare/3, except that it does not abort when asked to compare
+	% non-canonical terms.
+	%
+	% The declarative semantics of compare_representation for unequal
+	% non-canonical terms is that the result is either (<) or (>).  For
+	% equal non-canonical terms the result can be anything.
+	%
+	% Operationally, the result of compare_representation for
+	% non-canonical terms is the same as that for comparing the internal
+	% representations of the terms, where the internal representation is
+	% that which would be produced by deconstruct__cc.
 	%
 	% XXX This predicate is not implemented for highlevel code.  This
 	% is the reason it is not in the official part of the interface.
diff -u runtime/mercury_ho_call.c runtime/mercury_ho_call.c
--- runtime/mercury_ho_call.c
+++ runtime/mercury_ho_call.c
@@ -335,15 +335,15 @@
 
 #include "mercury_unify_compare_body.h"
 
-#undef  DECLARE_LOCALS
-#undef  initialize
-#undef  return_answer
+#undef	DECLARE_LOCALS
+#undef	initialize
+#undef	return_answer
 #undef	tailcall_user_pred
-#undef  start_label
+#undef	start_label
 #undef	call_user_code_label
-#undef  type_stat_struct
-#undef  attempt_msg
-#undef  entry_point_is_mercury
+#undef	type_stat_struct
+#undef	attempt_msg
+#undef	entry_point_is_mercury
 
 }
 
@@ -409,14 +409,14 @@
 
 #include "mercury_unify_compare_body.h"
 
-#undef  DECLARE_LOCALS
-#undef  initialize
-#undef  return_answer
+#undef	DECLARE_LOCALS
+#undef	initialize
+#undef	return_answer
 #undef	tailcall_user_pred
-#undef  start_label
+#undef	start_label
 #undef	call_user_code_label
-#undef  type_stat_struct
-#undef  attempt_msg
+#undef	type_stat_struct
+#undef	attempt_msg
 #undef	select_compare_code
 #undef	entry_point_is_mercury
 
@@ -457,18 +457,18 @@
 #define	type_stat_struct	MR_type_stat_mer_compare
 #define	attempt_msg		"attempt to compare representation "
 #define	select_compare_code
-#define include_compare_rep_code
+#define	include_compare_rep_code
 #define	entry_point_is_mercury
 
 #include "mercury_unify_compare_body.h"
 
-#undef  DECLARE_LOCALS
-#undef  initialize
-#undef  return_answer
-#undef  start_label
+#undef	DECLARE_LOCALS
+#undef	initialize
+#undef	return_answer
+#undef	start_label
 #undef	call_user_code_label
-#undef  type_stat_struct
-#undef  attempt_msg
+#undef	type_stat_struct
+#undef	attempt_msg
 #undef	select_compare_code
 #undef	include_compare_rep_code
 #undef	entry_point_is_mercury
@@ -511,14 +511,14 @@
 
 #include "mercury_unify_compare_body.h"
 
-#undef  DECLARE_LOCALS
-#undef  initialize
-#undef  return_answer
+#undef	DECLARE_LOCALS
+#undef	initialize
+#undef	return_answer
 #undef	tailcall_user_pred
-#undef  start_label
+#undef	start_label
 #undef	call_user_code_label
-#undef  type_stat_struct
-#undef  attempt_msg
+#undef	type_stat_struct
+#undef	attempt_msg
 }
 
 static MR_Word
@@ -555,14 +555,14 @@
 
 #include "mercury_unify_compare_body.h"
 
-#undef  DECLARE_LOCALS
-#undef  initialize
-#undef  return_answer
+#undef	DECLARE_LOCALS
+#undef	initialize
+#undef	return_answer
 #undef	tailcall_user_pred
-#undef  start_label
+#undef	start_label
 #undef	call_user_code_label
-#undef  type_stat_struct
-#undef  attempt_msg
+#undef	type_stat_struct
+#undef	attempt_msg
 #undef	select_compare_code
 }
 
@@ -588,17 +588,17 @@
 #define	type_stat_struct	MR_type_stat_c_compare
 #define	attempt_msg		"attempt to compare representation"
 #define	select_compare_code
-#define include_compare_rep_code
+#define	include_compare_rep_code
 
 #include "mercury_unify_compare_body.h"
 
-#undef  DECLARE_LOCALS
-#undef  initialize
-#undef  return_answer
-#undef  start_label
+#undef	DECLARE_LOCALS
+#undef	initialize
+#undef	return_answer
+#undef	start_label
 #undef	call_user_code_label
-#undef  type_stat_struct
-#undef  attempt_msg
+#undef	type_stat_struct
+#undef	attempt_msg
 #undef	select_compare_code
 #undef	include_compare_rep_code
 }
@@ -610,6 +610,10 @@
 	MR_Closure_Layout   *y_layout;
 	MR_Proc_Id          *x_proc_id;
 	MR_Proc_Id          *y_proc_id;
+	MR_ConstString      x_module_name;
+	MR_ConstString      y_module_name;
+	MR_ConstString      x_pred_name;
+	MR_ConstString      y_pred_name;
 	MR_TypeInfo         *x_type_params;
 	MR_TypeInfo         *y_type_params;
 	int                 x_num_args;
@@ -630,10 +634,40 @@
 
 	x_proc_id = &x_layout->MR_closure_id->MR_closure_proc_id;
 	y_proc_id = &y_layout->MR_closure_id->MR_closure_proc_id;
-	if (x_proc_id  < y_proc_id) {
-		return MR_COMPARE_LESS;
-	} else if (x_proc_id > y_proc_id) {
-		return MR_COMPARE_GREATER;
+
+	if (x_proc_id != y_proc_id) {
+		if (MR_PROC_ID_COMPILER_GENERATED(*x_proc_id)) {
+			x_module_name = x_proc_id->MR_proc_comp.
+						MR_comp_def_module;
+			x_pred_name = x_proc_id->MR_proc_comp.MR_comp_pred_name;
+		} else {
+			x_module_name = x_proc_id->MR_proc_user.
+						MR_user_decl_module;
+			x_pred_name = x_proc_id->MR_proc_user.MR_user_name;
+		}
+		if (MR_PROC_ID_COMPILER_GENERATED(*y_proc_id)) {
+			y_module_name = y_proc_id->MR_proc_comp.
+						MR_comp_def_module;
+			y_pred_name = y_proc_id->MR_proc_comp.MR_comp_pred_name;
+		} else {
+			y_module_name = y_proc_id->MR_proc_user.
+						MR_user_decl_module;
+			y_pred_name = y_proc_id->MR_proc_user.MR_user_name;
+		}
+
+		result = strcmp(x_module_name, y_module_name);
+		if (result < 0) {
+			return MR_COMPARE_LESS;
+		} else if (result > 0) {
+			return MR_COMPARE_GREATER;
+		}
+
+		result = strcmp(x_pred_name, y_pred_name);
+		if (result < 0) {
+			return MR_COMPARE_LESS;
+		} else if (result > 0) {
+			return MR_COMPARE_GREATER;
+		}
 	}
 
 	x_num_args = x->MR_closure_num_hidden_args;
diff -u tests/hard_coded/compare_representation.m tests/hard_coded/compare_representation.m
--- tests/hard_coded/compare_representation.m
+++ tests/hard_coded/compare_representation.m
@@ -39,12 +39,7 @@
 	io__write_string(SB),
 	io__nl,
 	{ std_util__compare_representation(Res, A, B) },
-	(
-		{ Res = (=) }
-	->
-		[]
-	;
-		io__write_string("maybe not ")
-	),
-	io__write_string("equal\n\n").
+	io__write_string("Result = "),
+	io__write(Res),
+	io__write_string(".\n\n").
 
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------