[m-dev.] for review: add type_ctor_reps for user-defined equality

[Fergus Henderson]

On 26-Oct-1999, Tyson Dowd <trd at cs.mu.OZ.AU> wrote:
> 
> Estimated hours taken: 4
> 
> Add support for distinguishing types with user defined equality from 
> system defined equality.  This information is encoded in the
> type_ctor_rep.
> 
> It is intended to be used for implementing RTTI driven compare and unify
> procedures.
> 
> This change uses the version number in the type_ctor_info to allow 
> Easy Bootstrapping(TM).
> 
> compiler/base_type_info.m:
> 	Add new alternatives for user defined equality types.
> 	Update RTTI version number.
> 	Change `type_ctor_info_version' to `type_ctor_info_rtti_version'.
> 
> runtime/mercury_type_info.c:
> 	Define MR_get_new_type_ctor_rep which gets the type_ctor_rep,
> 	automatically converting older versions.
> 	(this change is just for bootstrapping).

It would be a good idea to add a comment pointing to here from
the definition of MR_GRADE_PART_0 in runtime/mercury_grade.h,
e.g.

	/*
	** Note: next time the binary compatibility version
	** number is increased, we should remove the
	** MR_get_new_type_ctor_rep() function in
	** runtime/mercury_type_info.c.
	*/

so that we don't forget to remove this "bootstrapping only" code
when the time comes.

> library/std_util.m:
> runtime/mercury_deep_copy_body.h:
> runtime/mercury_ho_call.c:
> runtime/mercury_tabling.c:
> runtime/mercury_type_info.h:
> runtime/mercury_wrapper.c:
> 	Use MR_get_new_type_ctor_rep to get the type_ctor_rep.
> 	(this change is just for bootstrapping).
> 
> 	Add new alternatives to code that uses type_ctor_rep (at the
> 	moment they just fall through).

I just noticed that the code in runtime/mercury_ho_call.c that Zoltan
added recently does not handle enums with user-defined equality
correctly; it compares them using structural equality, rather than
by calling the user-defined equality predicate.  Your change preserves
this bug.  Instead, you should fix the bug by treating ENUM_USEREQ the
same as DU_USEREQ, i.e. calling the unify/compare/index pred,
rather than treating it the same as ENUM.

> --- mercury_deep_copy_body.h	1999/10/19 04:11:40	1.13
> +++ mercury_deep_copy_body.h	1999/10/25 03:38:22
> @@ -31,7 +31,6 @@
>  
>      Word functors_indicator;
>      Word layout_entry, *entry_value, *data_value;
> -    MR_TypeCtorRepresentation type_ctor_rep;
>      MR_DiscUnionTagRepresentation tag_rep;
>      int data_tag; 
>      Word new_data, data;
> @@ -45,7 +44,7 @@
>      layout_entry = type_ctor_info->type_ctor_layout[data_tag];
>      entry_value = (Word *) MR_strip_tag(layout_entry);
>  
> -    switch (type_ctor_info->type_ctor_rep) {
> +    switch (MR_get_new_type_ctor_rep(type_ctor_info)) {
>          case MR_TYPECTOR_REP_ENUM:
>              new_data = data;    /* just a copy of the actual item */
>              break;

Don't you need to add new cases for *_USEREQ here?

> Index: runtime/mercury_ho_call.c
...
> @@ -288,8 +289,10 @@
>  			*/
>  
>  		case MR_TYPECTOR_REP_DU:
> +		case MR_TYPECTOR_REP_DU_USEREQ:
>  		case MR_TYPECTOR_REP_ARRAY:
>  		case MR_TYPECTOR_REP_NOTAG:
> +		case MR_TYPECTOR_REP_NOTAG_USEREQ:
>  		case MR_TYPECTOR_REP_EQUIV:
>  		case MR_TYPECTOR_REP_EQUIV_VAR:
>  
> @@ -355,6 +358,7 @@
>  				LABEL(mercury__unify_2_0));
>  
>  		case MR_TYPECTOR_REP_ENUM:
> +		case MR_TYPECTOR_REP_ENUM_USEREQ:

ENUM_USEREQ should be handle with DU_USEREQ not with ENUM.

> @@ -504,10 +508,11 @@
>  	type_ctor_info = MR_TYPEINFO_GET_TYPE_CTOR_INFO((Word *) type_info);
>  
>  #ifdef	MR_CTOR_REP_STATS
> -	MR_ctor_rep_index[type_ctor_info->type_ctor_rep]++;
> +	MR_ctor_rep_index[MR_get_new_type_ctor_rep(
> +				type_ctor_info->type_ctor_rep)]++;
>  #endif
>  
> -	switch (type_ctor_info->type_ctor_rep) {
> +	switch (MR_get_new_type_ctor_rep(type_ctor_info)) {

Don't you need to add cases to that switch to handle index/2 of
*_USEREQ?

> @@ -699,10 +704,11 @@
>  	type_ctor_info = MR_TYPEINFO_GET_TYPE_CTOR_INFO((Word *) type_info);
>  
>  #ifdef	MR_CTOR_REP_STATS
> -	MR_ctor_rep_compare[type_ctor_info->type_ctor_rep]++;
> +	MR_ctor_rep_compare[MR_get_new_type_ctor_rep(
> +				type_ctor_info->type_ctor_rep)]++;
>  #endif
>  
> -	switch (type_ctor_info->type_ctor_rep) {
> +	switch (MR_get_new_type_ctor_rep(type_ctor_info)) {

Don't you need to add cases to that switch to handle compare/3 of
*_USEREQ?

> Index: runtime/mercury_tabling.c
...
> -    switch (type_ctor_info->type_ctor_rep) {
> +    switch (MR_get_new_type_ctor_rep(type_ctor_info)) {
>          case MR_TYPECTOR_REP_ENUM: {
>              int functors = MR_TYPE_CTOR_LAYOUT_ENUM_VECTOR_NUM_FUNCTORS(
>                                  layout_vector_for_tag);

Don't you need to add cases to that switch to handle *_USEREQ?

> +++ mercury_type_info.h	1999/10/22 04:16:59
> @@ -772,16 +773,20 @@
>  
>  /*
>  ** MR_DataRepresentation is the representation for a particular type
> -** constructor.  For the cases of MR_TYPE_CTOR_REP_DU the exact
> -** representation depends on the tag value -- lookup the tag value in
> -** type_ctor_layout to find out this information.
> +** constructor.  For the cases of MR_TYPE_CTOR_REP_DU and
> +** MR_TYPE_CTOR_REP_DU_USEREQthe exact representation depends on the tag
> +** value -- lookup the tag value in type_ctor_layout to find out this
> +** information.

s/USEREQthe/USEREQ, the/

> +typedef enum MR_OldTypeCtorRepresentation {
> +	MR_OLD_TYPECTOR_REP_ENUM,
> +	MR_OLD_TYPECTOR_REP_DU,
> +	MR_OLD_TYPECTOR_REP_NOTAG,
> +	MR_OLD_TYPECTOR_REP_EQUIV,
> +	MR_OLD_TYPECTOR_REP_EQUIV_VAR,
> +	MR_OLD_TYPECTOR_REP_INT,
> +	MR_OLD_TYPECTOR_REP_CHAR,
> +	MR_OLD_TYPECTOR_REP_FLOAT,
> +	MR_OLD_TYPECTOR_REP_STRING,
> +	MR_OLD_TYPECTOR_REP_PRED,
> +	MR_OLD_TYPECTOR_REP_UNIV,
> +	MR_OLD_TYPECTOR_REP_VOID,
> +	MR_OLD_TYPECTOR_REP_C_POINTER,
> +	MR_OLD_TYPECTOR_REP_TYPEINFO,
> +	MR_OLD_TYPECTOR_REP_TYPECLASSINFO,
> +	MR_OLD_TYPECTOR_REP_ARRAY,
> +	MR_OLD_TYPECTOR_REP_SUCCIP,
> +	MR_OLD_TYPECTOR_REP_HP,
> +	MR_OLD_TYPECTOR_REP_CURFR,
> +	MR_OLD_TYPECTOR_REP_MAXFR,
> +	MR_OLD_TYPECTOR_REP_REDOFR,
> +	MR_OLD_TYPECTOR_REP_REDOIP,
> +	MR_OLD_TYPECTOR_REP_TRAIL_PTR,
> +	MR_OLD_TYPECTOR_REP_TICKET,
> +	/*
> +	** MR_OLD_TYPECTOR_REP_UNKNOWN should remain the last alternative;
> +	** MR_CTOR_REP_STATS depends on this.
> +	*/
> +	MR_OLD_TYPECTOR_REP_UNKNOWN
> +} MR_OldTypeCtorRepresentation;

Is that stuff referenced anywhere?
If not, why keep it?

There should at least be a comment here explaining what this is for, IMHO.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------