[m-rev.] for review: avoid using some "keywords" 5

Julien Fischer jfischer at opturion.com
Mon May 16 10:26:30 AEST 2016


Hi Zoltan,

On Sat, 14 May 2016, Zoltan Somogyi wrote:

> I have wanted to make this change for a long time,
> whenever I got the confusing error messages the log
> message refers to, but such times are the *only* times
> when you cannot make this change: if you do it in the
> same workspace, you make your half-done job harder,
> while if you do it an another workspace, you are guaranteed
> merge conflicts.
>
> The only part that needs review is the naming scheme
> for the replacement function symbols in prog_data.m
> and hlds_goal.m: do people agree that they are an
> improvement over the old state of things?
>
> Avoid using some Mercury keywords.
> 
> Unifications (x = y) have long had two descriptions of their modes.
> One is the unify_mode, which used to look like this:
>     (initx -> finalx) - (inity -> finaly)
> and other is the uni_mode, which used to look like this:
>     (initx - inity) -> (finalx - finaly)
> Each unification has one unify_mode, and the unifications that includes
> a function symbol has one uni_mode per argument of that function symbol.

That last sentence doesn't parse.

...

> diff --git a/compiler/hlds_goal.m b/compiler/hlds_goal.m
> index 889f755..a2664df 100644
> --- a/compiler/hlds_goal.m
> +++ b/compiler/hlds_goal.m
> @@ -1052,13 +1052,18 @@
>      --->    cell_is_unique
>      ;       cell_is_shared.
> 
> -:- type unify_mode == pair(mer_mode, mer_mode).
> +:- type unify_mode
> +    --->    unify_modes_lhs_rhs(mer_mode, mer_mode).
> +
> +:- type uni_insts
> +    --->    uni_insts_lhs_rhs(mer_inst, mer_inst).
> +            % Respectively, the insts of the left and right hand sides
> +            % of the unification.
>  :- type uni_mode
> -    --->    pair(mer_inst) -> pair(mer_inst).
> -            % Each uni_mode maps a pair of insts to a pair of new insts
> -            % Each pair represents the insts of the LHS and the RHS
> -            % respectively.
> +    --->    from_to_uni_insts(uni_insts, uni_insts).
> +            % The initial insts of the two sides of the unification,
> +            % and their final insts.

I would prefer not to abbreviate "unify" or "unification" to "uni" as in a
system that also deals with "unique" or "uniqueness" all over the place, it's
confusing.  I suggest the following for the above:

     :- type unify_modes
         --->    unify_modes_lhs_rhs(mer_mode, mer_mode).

     :- type unify_insts
         --->    unify_insts_lhs_rhs(mer_inst, mer_inst).

     :- type unify_mode
         --->    from_to_unify_insts(unify_insts, unify_insts).

The rest of the diff looks ok.

Julien.


More information about the reviews mailing list