[m-rev.] for post-commit review: centralize decisions about mutables' aux preds

[Julien Fischer]

Hi Zoltan,

On Sun, 1 Sep 2019, Zoltan Somogyi wrote:

> Centralize decisions about which aux preds a mutable needs.

...


> diff --git a/compiler/add_pred.m b/compiler/add_pred.m
> index 35f1b08dd..67f139ea4 100644
> --- a/compiler/add_pred.m
> +++ b/compiler/add_pred.m

...

> diff --git a/compiler/prog_data.m b/compiler/prog_data.m
> index a2274fdc7..57b1de132 100644
> --- a/compiler/prog_data.m
> +++ b/compiler/prog_data.m
> @@ -1461,15 +1461,50 @@ best_purity(purity_impure, purity_impure) = purity_impure.
>
>  :- interface.
> 
> +    % The kinds of auxiliary predicates we may need to generate
> +    % to implement a mutable.
> +    %
> +    % The first group represent the public predicates, the predicates
> +    % that user programs may call. The usual (non-constant) kind of mutable
> +    % will have the standard get and set predicates, and if attached
> +    % to the I/O state, will have the I/O get and set predicates as well.
> +    % Constant mutables will have the constant get and set predicates instead
> +    % (see below).
> +    %
> +    % The first group represent the private predicates, the predicates

s/first/second/ there.

> +    % that user programs should not call (and which are not documented).
> +    % The unsafe get and set predicates may be needed to implement the other,

s/may/are/

> +    % user-visible get and set predicates, and the lock and unlock predicates
> +    % have the same role. The initialization predicate is called by the
> +    % implementation itself at program startup, and it may need the help
> +    % of the preinit predicate.

...

> diff --git a/library/set.m b/library/set.m
> index 27041df64..5df901712 100644
> --- a/library/set.m
> +++ b/library/set.m
> @@ -150,14 +150,20 @@
>      % containing only `X', i.e.  if `Set' is the set which contains
>      % all the elements of `Set0' except `X'.
>      %
> +    % The det_remove version aborts if the removal fails.

We generally don't describe library predicates as aborting anymore.
For one thing it's ambiguous: does it throw an exception or do something
else (e.g. call MR_fatal_error)?

I would reword that as as:

     The det_version throws an exception instaed of failing.

> +    %
>  :- pred remove(T::in, set(T)::in, set(T)::out) is semidet.
> +:- pred det_remove(T::in, set(T)::in, set(T)::out) is det.
>
>      % `remove_list(Xs, Set0, Set)' is true iff `Xs' does not
>      % contain any duplicates, `Set0' contains every member of `Xs',
>      % and `Set' is the relative complement of `Set0' and the set
>      % containing only the members of `Xs'.
>      %
> +    % The det_remove_list version aborts if any removal fails.
> +    %


Ditto.

The rest looks fine.

Julien.