[m-dev.] for review: merge HAL branch onto main branch

Fergus Henderson fjh at cs.mu.OZ.AU
Fri Feb 23 17:29:06 AEDT 2001


On 23-Feb-2001, David Jeffery <dgj at cs.mu.OZ.AU> wrote:
> @@ -2055,6 +2058,12 @@
>                  "\tThis is necessary for interfacing with constraint solvers,",
>                  "\tor for backtrackable destructive update.",
>                  "\tThis option is not yet supported for the IL or Java back-ends.",
> +                "--reserve-tag\t\t\t(grade modifier: `.rt')",
> +                "\tReserve a tag in the data representation of the generated ",
> +                "\tcode. This tag is intended to be used to give an explicit",
> +                "\treprestation to free variables.",

s/represtation/representation/

> +++ compiler/type_ctor_info.m	2001/02/23 04:53:31
> @@ -399,12 +397,19 @@
>  % Make the functor and notag tables for an enum type.
>  
>  :- pred type_ctor_info__make_enum_tables(list(constructor)::in,
> -        cons_tag_values::in, rtti_type_id::in, list(rtti_data)::out,
> +        cons_tag_values::in, rtti_type_id::in, bool::in, list(rtti_data)::out,
>          type_ctor_functors_info::out, type_ctor_layout_info::out) is det.
>  
> -type_ctor_info__make_enum_tables(Ctors, ConsTagMap, RttiTypeId,
> +type_ctor_info__make_enum_tables(Ctors, ConsTagMap, RttiTypeId, ReserveTag,
>                  TypeTables, FunctorInfo, LayoutInfo) :-
> -        type_ctor_info__make_enum_functor_tables(Ctors, 0, ConsTagMap,
> +        (
> +                ReserveTag = yes
> +        ->
> +                error("type_ctor_info__make_enum_tables: no enums in .rt grade")

It would be better to use unexpected/2 from error_util.m here.

> +:- pred type_is_single_ctor_single_arg(list(constructor), sym_name, 
> +        maybe(sym_name), type).
> +:- mode type_is_single_ctor_single_arg(in, out, out, out) is semidet.
> +
> +type_is_single_ctor_single_arg(Ctors, Ctor, MaybeSymName, ArgType) :-
> +        Ctors = [SingleCtor],
> +        SingleCtor = ctor(ExistQVars, _Constraints, Ctor, 
> +                [MaybeSymName - ArgType]),
> +        ExistQVars = [].

A comment here explaining the `ExistQVars = []' test would help,
e.g.
	
	% If there are any existentially quantified type variables,
	% then the type will contain hidden fields holding the type_infos
	% and/or typeclass infos for those type variables,
	% so it won't be a single-argument type.

> Index: compiler/unify_proc.m
> ===================================================================
> RCS file: /home/staff/zs/imp/mercury/compiler/unify_proc.m,v
> retrieving revision 1.91
> diff -u -t -r1.91 unify_proc.m
> --- compiler/unify_proc.m	2000/11/06 08:28:40	1.91
> +++ compiler/unify_proc.m	2001/02/12 03:30:07
> @@ -719,8 +719,22 @@
>                          unify_proc__quantify_clauses_body([H1, H2], Goal,
>                                  Context, Clauses)
>                  ;
> -                        unify_proc__generate_du_unify_clauses(Ctors,
> -                                H1, H2, Context, Clauses)
> +                        % Check to see if it's a single zero-arity functor.
> +                        % This hack is required for --reserve-tag, which
> +                        % disables enumeration types, so that the compiler
> +                        % does not warn about inferring the unify to be
> +                        % det rather than semidet.
> +                        ( { Ctors = [ctor(_ExistQVars, _Constraints, _FunctorName, [])] , semidet_fail } ->
> +                                % We must at least pretend a unification is
> +                                % required?
> +                                { create_atomic_unification(H1, var(H2),
> +                                        Context, explicit, [], Goal) },
> +                                unify_proc__quantify_clauses_body([H1, H2],
> +                                        Goal, Context, Clauses)
> +                        ;
> +                                unify_proc__generate_du_unify_clauses(Ctors,
> +                                        H1, H2, Context, Clauses)
> +                        )

The call to semidet_fail means that that code is disabled.
Did you intend to commit this bit as is?

The documentation should explain why the code is disabled.
There should probably be an XXX.
The condition there should be written over multiple lines.

> +#ifdef MR_RESERVE_TAG

That configuration macro should be documented in
runtime/mercury_conf_param.h.

> +++ runtime/mercury_tags.h	2001/02/21 05:17:48
>  
> +#ifdef MR_RESERVE_TAG
> +    #define MR_RAW_TAG_VAR               0     /* for Prolog-style variables */
> +    #define MR_FIRST_UNRESERVED_RAW_TAG  1
> +#else
> +    #define MR_FIRST_UNRESERVED_RAW_TAG  0
> +#endif

That should be indented two spaces rather than four.

> +#define MR_RAW_TAG_CONS         MR_FIRST_UNRESERVED_RAW_TAG + 1

The bodies of macro definitions like that should be parenthesized.

> +++ runtime/mercury_type_info.h	2001/02/23 05:01:17
> @@ -347,9 +347,22 @@
>  ** Definitions for handwritten code, mostly for mercury_compare_typeinfo.
>  */
>  
> -#define MR_COMPARE_EQUAL    0
> -#define MR_COMPARE_LESS     1
> -#define MR_COMPARE_GREATER  2
> +#ifdef MR_RESERVE_TAG
> +        /*
> +        ** In reserve-tag grades, enumerations are disabled, so the
> +        ** representation of the 'comparison_result' type is quite different.
> +        ** The enumeration constants (for (<), (=) and (>)) wind up sharing 
> +        ** the same primary tag (1), and are all allocated secondary tags
> +        ** starting from 0.
> +        */
> +    #define MR_COMPARE_EQUAL    ((0 << LOW_TAG_BITS) + 1)
> +    #define MR_COMPARE_LESS     ((1 << LOW_TAG_BITS) + 1)
> +    #define MR_COMPARE_GREATER  ((2 << LOW_TAG_BITS) + 1)

You should write that using the MR_mkword(), MR_mktag(), and
MR_mkbody() macros from runtime/mercury_tags.h, rather than via
explicit bit shifting:

    #define MR_COMPARE_EQUAL    MR_mkword(MR_mktag(1), MR_mkbody(0))
    #define MR_COMPARE_LESS     MR_mkword(MR_mktag(1), MR_mkbody(1))
    #define MR_COMPARE_GREATER  MR_mkword(MR_mktag(1), MR_mkbody(2))

Or perhaps even avoiding the magic number 1:

    #define MR_COMPARE_TAG	MR_mktag(MR_FIRST_UNRESERVED_RAW_TAG)

    #define MR_COMPARE_EQUAL    MR_mkword(MR_COMPARE_TAG, MR_mkbody(0))
    #define MR_COMPARE_LESS     MR_mkword(MR_COMPARE_TAG, MR_mkbody(1))
    #define MR_COMPARE_GREATER  MR_mkword(MR_COMPARE_TAG, MR_mkbody(2))

----------

The new `.rt' grade modifier should be documented in the help message
for the `--grade' option in compiler/options.m.

The new option and grade modifier should be documented in
doc/user_guide.texi too, as well as in the help message in
compiler/options.m.

According to our C coding guidelines, the MR_define_univ_fields(),
MR_unravel_univ(), and MR_new_univ_on_hp() macros should all be
uppercase, since they're not function-like: they modify their arguments.
But don't worry about that one, MR_define_univ_fields() was that way
before your change.  We can fix that one as a separate change.

Apart from that, this looks fine.

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



More information about the developers mailing list