[m-rev.] for review: split up term.m

Julien Fischer jfischer at opturion.com
Sun Feb 1 23:13:14 AEDT 2015


Hi Zoltan,

On Wed, 28 Jan 2015, Zoltan Somogyi wrote:

> We have already discussed the split-up, but I would like review
> comments, from anyone, about the name of the new module,
> term_conversion.m. (Note that only four modules in the entire
> Mercury system import it, so splitting it off seems like a good idea.)

Either that or just term_conv.m would be fine.

Another change that may also be worth considering is shifting the context/0
type into its own module.  In the Mercury system itself, I suspect there are
quite a few modules importing term.m solely because they require the definition
of the context/0 type.  More generally, "contexts" are things that are useful
in contexts not related to Herbrand terms.

> I would also like comments about how we should go about fixing
> the problems I noted in term.m. Since the reordering in term.m
> yields a diff that is bigger than term.m, I have attached the
> new term.m as well. It has a bunch of XXXs noting problems,
> which fall into three main categories:
>
> - Many predicates have argument orders that predate state var notation
>  and don't work well with it.
> - Some have names that are simply not very meaningful.
> - And some predicates duplicate the functionality of other predicates.
>
> None can be fixed without breaking backward compatibility.

I'm not concerned that backwards compatibility will break here, but I do have a
preference as to _when_ it is broken.  The reason is that Cadmium's rule parser
uses the term representation and Opturion also has a policy of requiring our
code to compile with the current Mercury stable release (e.g. 14.01.1).  If any
changes to the term module that break backwards compatibility are delayed until
just before we are ready to branch for the next release then that's fine by me.
(At that point, all the Opturion stuff will be updated to make use of things
in the new version of the stdlib anyway.)

> I propose that we fix the second and third problems by picking a good name
> for each predicate, whether it is an existing name for the job or not, moving
> the implementation to that name, make any existing predicates that do that
> job forward to the one with the good name, and mark the old predicate names
> as obsolete.

Agreed.

> If the good names are new names, then we can make the new
> predicate use a state-var-friendly order of arguments. I think
> we can do this in most or all cases, by picking names that explicitly
> name *what* they work on. For example, instead of apply_renaming,
> make it apply_renaming_to_term, and instead of apply_renaming_to_list,
> make it apply_renaming_to_terms.
>
> I don't intend to commit the attached diff; I intend to commit only
> an updated version that resolves these issues. For that, I await
> your comments.

One comment concerning the diff: s/Rebame/Rename/

Cheers,
Julien.



More information about the reviews mailing list