[m-rev.] for review: improve doc and error messages for existentially typed data

[Julien Fischer]

Hi Zoltan,

On Wed, 6 May 2020, Zoltan Somogyi wrote:

> Improve doc and error messages for existential types.
> 
> doc/reference_manual.texi:
>     Document the restrictions that the parse_type_defn.m implements
>     (or at least tries to).
>
>     Fix two bugs in one of the "supposed to be correct" examples,
>     as well as the formatting that previously made the bugs harder to see.
> 
> compiler/parse_type_defn.m:
>     Improve the wording of error messages for errors involving existentially
>     typed data constructors.
>
>     Simplify the structure of one of the predicates that generate such
>     messages.
> 
> tests/invalid/bad_existential_data_type.{m,err_exp}:
>     A new test case for the updated error messages, most of which
>     weren't being tested before.
> 
> tests/invalid/Mmakefile:
>     Enable the new test case.
> 
> tests/invalid/fundeps_unbound_in_ctor.err_exp:
> tests/invalid/type_vars.err_exp:
>     Update the expected wording of some error messages.
> 
> diff --git a/compiler/parse_type_defn.m b/compiler/parse_type_defn.m
> index 59f5fc2bd..2a14ad3b2 100644
> --- a/compiler/parse_type_defn.m
> +++ b/compiler/parse_type_defn.m

...

> @@ -584,14 +587,14 @@ process_du_ctors(Params, VarSet, BodyTerm, [Ctor | Ctors], !Specs) :-
>          NotOccursExistQVarStrs =
>              list.map(mercury_var_to_name_only(GenericVarSet),
>              NotOccursExistQVars),
> -        Pieces = [words("Error:"),
> +        Pieces = [words("Error: the existentialy quantified"),

s/existentialy/existentially/
(And also in the expected output for the test of this message).

>              words(choose_number(NotOccursExistQVars,
>                  "type variable", "type variables"))] ++
>              list_to_quoted_pieces(NotOccursExistQVarStrs) ++
> -            [words("in existential quantifier"),
> -            words(choose_number(NotOccursExistQVars,
> +            [words(choose_number(NotOccursExistQVars,
>                  "does not occur", "do not occur")),
> -            words("in arguments or constraints of constructor."), nl],
> +            words("either in the arguments or in the constraints"),
> +            words("of the constructor."), nl],
>          Spec = simplest_spec($pred, severity_error, phase_term_to_parse_tree,
>              get_term_context(BodyTerm), Pieces),
>          !:Specs = [Spec | !.Specs]

...

> diff --git a/doc/reference_manual.texi b/doc/reference_manual.texi
> index 7b74a9299..8dfc9b706 100644
> --- a/doc/reference_manual.texi
> +++ b/doc/reference_manual.texi
> @@ -6230,7 +6230,7 @@ or conjunctions of type variables separated by commas.
>
>  Each type variable must appear in the parameter list of the typeclass.
>  Abstract typeclass declarations must have
> -exactly the same functional dependencies as in the implementation.
> +exactly the same functional dependencies as their concrete forms.
>
>  Mutually recursive functional dependencies are allowed,
>  so the following examples are legal:
> @@ -6661,18 +6661,15 @@ For example:
>      --->    some [T] (s(T) => showable(T)).
>  :- type list_of_showable == list(showable).
> 
> -% Here's an arbitrary example involving multiple
> -% type variables and multiple constraints.
> +% Here is an arbitrary example involving multiple type variables
> +% and multiple constraints.
>  :- typeclass foo(T1, T2) where [ /* @dots{} */ ].
>  :- type bar(T)
>      --->    f1
>      ;       f2(T)
> -    ;       some [T]
> -            f4(T)
> -    ;       some [T1, T2]
> -            (f4(T1, T2, T) => showable(T1), showable(T2))
> -    ;       some [T1, T2]
> -            (f5(list(T1), T2) => fooable(T1, list(T2))).
> +    ;       some [T1] f3(T1)
> +    ;       some [T1, T2] f4(T1, T2, T) => (showable(T1), showable(T2))
> +    ;       some [T1, T2] f5(list(T1), T2) => fooable(T1, list(T2)).
>  @end example

That last case is using behaviour we don't yet support, the constraint
fooable(T1, list(T2)) will result in the error:

    Sorry, not implemented: constraints may only constrain type
    variables and ground types

I suggest replacing it with something the current compiler will acccept.

The rest of the diff looks fine.

Julien.