[m-rev.] for review: lookup_name_default_prefix
Julien Fischer
jfischer at opturion.com
Mon May 29 18:53:37 AEST 2023
On Mon, 29 May 2023, Zoltan Somogyi wrote:
> Add lookup_name_default_prefix to varset.m.
>
> library/varset.m:
> We have long had two versions of the lookup_name function, and of
> the equivalent lookup_name predicate, with the difference being
> that one version had an extra argument that specified to prefix
> for the names to be constructed for not-explicitly-named variables.
>
> Obsolete the function and the predicate with this extra argument
> in favor of a new function/predicate pair, whose shared name is
> lookup_name_default_prefix.
>
> Act on an old sortof-XXX by changing the order of arguments of the
> set_bindings predicate to make it state-variable-friendly.
...
> diff --git a/NEWS.md b/NEWS.md
> index 1701e14d5..59d5b77be 100644
> --- a/NEWS.md
> +++ b/NEWS.md
> @@ -1800,10 +1800,21 @@ Changes to the Mercury standard library
>
> * The following functions and predicates have been added:
>
> + - func `lookup_name_default_prefix/3`
> + - pred `lookup_name_default_prefix/4`
> - func `unname_var/2`
> - pred `unname_var/3`
> - pred `undo_default_names/2`
>
> +* The following functions and predicates have been marked obsolete:
function and predicate
> + - func `lookup_name/3` (replacement: `lookup_name_default_prefix/3`)
> + - pred `lookup_name/4` (replacement: `lookup_name_default_prefix/4`)
> +
...
> diff --git a/library/term.m b/library/term.m
> index 3800694fd..bc23fb1f6 100644
> --- a/library/term.m
> +++ b/library/term.m
> @@ -117,10 +117,11 @@
>
> %---------------------------------------------------------------------------%
>
> - % from_int/1 should only be applied to integers returned by to_int/1.
> - % NOTE_TO_IMPLEMENTORS This instance declaration is needed to allow
> - % NOTE_TO_IMPLEMENTORS sets of variables to be represented using
> - % NOTE_TO_IMPLEMENTORS sparse_bitset.m and the other bitset modules.
> + % from_int/1 should only be applied to integers returned by to_int/1, and
> + % from_uint/1 should only be applied to integers returned by to_uint/1.
> + % NOTE_TO_IMPLEMENTORS The uenum instance declaration is needed to allow
> + % note_to_implementors sets of variables to be represented using
> + % note_to_implementors sparse_bitset.m and the other bitset modules.
You've lowercased note_to_implementors on the last two lines there.
> :- instance enum(var(_)).
> :- instance uenum(var(_)).
> diff --git a/library/varset.m b/library/varset.m
> index 28ca34894..174984708 100644
> --- a/library/varset.m
> +++ b/library/varset.m
> @@ -10,10 +10,10 @@
> % Main author: fjh.
> % Stability: low.
> %
> -% This file provides facilities for manipulating collections of variables.
> -% through the 'varset' ADT. These variables are object-level variables,
> -% and are represented as ground terms, so it might help to think of them
> -% as "variable ids" rather than Prolog-style variables.
> +% This file provides facilities for manipulating collections of variables,
> +% through the 'varset' abstract data type. These variables are object-level
> +% variables, and are represented as ground terms, so the right way to think
> +% of them is as "variable ids" rather than Prolog-style variables.
> %
> % A varset may record a name and/or a value (binding) with each variable.
> %
> @@ -31,9 +31,11 @@
> % In situations in which this is not a concern, programmers may use
> % the standard generic varset instance.
> %
> -% Note that there are some design flaws in the relationship between
> -% varset.m and term.m. There is too much coupling between the two,
> -% which may and should be fixed later, e.g. by merging the two modules.
> +% Note that varset.m and term.m are strongly coupled together, meaning that
> +% they each need the other. The reason why they have not been merged into
> +% one larger module is that many user modules call predicates and functions
> +% from just one of these two modules, even though, through that one module,
> +% they implicily depend on the other as well.
s/implicily/implicitly/
That looks fine otherwise.
Julien.
More information about the reviews
mailing list