[m-rev.] for review: carve four modules out of term.m

Julien Fischer jfischer at opturion.com
Sat Aug 20 15:44:44 AEST 2022

On Sat, 20 Aug 2022, Zoltan Somogyi wrote:

> I will write up the changes in NEWS after the review,
> and likewise, I will make any needed updates in extras
> or samples then, in both cases so I don't have to redo
> any changes requested by a review.
> I also intend to look into some way that compiler modules
> can get access to contexts without importing term.m
> This could be done by moving the type, and its operations,
> to a new module in the library, with an equivalence type
> as well as forwarding predicates left behind in term.m.

We have certainly discussed the possibility of moving the context type
to its own module before -- although, I could not find where on the
mailing list. From memory, there are quite large number of modules in
the compiler that are only importing the term module solely to get
access to the context type.

> Or it could be by adding a type functionally equivalent
> to term.context to the HLDS, and having the code
> that adds parse tree components to the HLDS convert
> contexts to this new type. The former approach is simpler,
> while the second is more flexible, in that it would allow us
> in the future to replace our current contexts, which always
> refer to a single line, with contexts that refer to a range
> of line numbers.
> On the other hand, adding column number info as well, e.g. to enable
> highlighting of just the characters of an erroneous term, would
> require changes in standard library modules anyway.
> Opinions?

As a starting point, I would move context to its own module, e.g.

> Carve four modules out of term.m.
> Most modules that imported the old term.m need only a small subset
> of its functionality. After this diff, most modules that used to import
> term.m will need to import just one more module, and will import many
> fewer predicates and functions in total.


> diff --git a/library/term.m b/library/term.m
> index e7c009243..0ab937029 100644
> --- a/library/term.m
> +++ b/library/term.m
> @@ -138,131 +138,126 @@
>  :- type substitution(T) == map(var(T), term(T)).
>  :- type substitution    == substitution(generic).
> +%---------------------------------------------------------------------------%
> +
> +    % generic_term(Term) is true iff Term is a term of type
> +    % `term' i.e. `term(generic)'. It is useful because in some instances
> +    % it doesn't matter what the type of a term is, and passing it to this
> +    % predicate will ground the type avoiding unbound type variable warnings.
> +    % NOTE_TO_IMPLEMENTORS XXX This is not all that useful,
> +    % NOTE_TO_IMPLEMENTORS since we now have with_type.
> +    %
> +:- pred generic_term(term::in) is det.

Perhaps we can just mark this as obsolete?


> diff --git a/library/term_int.m b/library/term_int.m
> index e69de29bb..a0ffc1b92 100644
> --- a/library/term_int.m
> +++ b/library/term_int.m
> @@ -0,0 +1,176 @@
> +%---------------------------------------------------------------------------%
> +% vim: ts=4 sw=4 et ft=mercury
> +%---------------------------------------------------------------------------%
> +% Copyright (C) 1993-2000,2003-2009,2011-2012 The University of Melbourne.
> +% Copyright (C) 2014-2022 The Mercury team.
> +% This file is distributed under the terms specified in COPYING.LIB.
> +%---------------------------------------------------------------------------%
> +%
> +% File: term_in.m.
> +% Main author: fjh.
> +% Stability: medium.
> +%
> +% This file provides ways to test whether terms represent integers,
> +% and ways to construct terms representing integers.
> +%
> +%---------------------------------------------------------------------------%
> +%---------------------------------------------------------------------------%
> +
> +:- module term_int.
> +:- interface.
> +
> +:- import_module term.
> +
> +%---------------------------------------------------------------------------%
> +
> +:- pred decimal_term_to_int(term(T)::in, int::out) is semidet.
> +% XXX NOTE_TO_IMPLEMENTORS Why no versions for sized and/or unsigned ints?

decimal_term_to_int/2 is used within the compiler when processing terms
containing numbers (e.g. arities in instance or pragma decls). All of
those places only require decimal ints and do not use the other integer
types (i.e. there's never been any need to have similar operation for
other types.)

IIRC, before the other integer types were added the code in the compiler
was actually incorrect and would have allowed, e.g. hexadecimal arities,
when it was parsing terms that referred to arities.  the change to the
term module's representation of integers made this show up a bit more
obviously, which was why decimal_term_to_int was added.

The diff looks fine.


More information about the reviews mailing list