[m-rev.] for post-commit review: propagate type char into insts

Julien Fischer jfischer at opturion.com
Tue Jun 8 15:21:29 AEST 2021



On Tue, 8 Jun 2021, Zoltan Somogyi wrote:

> Propagate type char into bound_insts.
> 
> compiler/mode_util.m:
>     As above. This is needed to allow the compiler to check whether
>     a char constant matches an inst that is specific to the char type.
> 
> tests/invalid/char_inst.{m,err_exp}:
>     A new test case for the fixed functionality.
> 
> tests/invalid/Mmakefile:
>     Enable the new test case.

...

> diff --git a/tests/invalid/char_inst.m b/tests/invalid/char_inst.m
> index e69de29bb..406ec0c4a 100644
> --- a/tests/invalid/char_inst.m
> +++ b/tests/invalid/char_inst.m
> @@ -0,0 +1,36 @@
> +%---------------------------------------------------------------------------%
> +% vim: ts=4 sw=4 et ft=mercury
> +%---------------------------------------------------------------------------%
> +
> +:- module char_inst.
> +
> +:- interface.
> +:- import_module io.
> +
> +:- pred main(io::di, io::uo) is det.
> +
> +:- implementation.
> +:- import_module char.
> +
> +main(!IO) :-
> +    test('s', !IO),     % This should be accepted without complaint.
> +    test('x', !IO).     % This should generate an error message.
> +
> +:- inst key_char for char/0
> +    --->    ('s')
> +    ;       ('i')
> +    ;       ('f').


That test could be stronger, for example:

     :- inst key_char for char/0
         --->    ('s')
         ;       b
         ;       0'a
         ;       0'Δ
         ;       ('\n').

(I'm not sure if the fouth one is something we actually support
or something that just happens to work.)

You should add an entry to the NEWS file saying that insts for chars
are now supported.

The diff itself looks fine.

Julien.


More information about the reviews mailing list