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

Julien Fischer jfischer at opturion.com
Tue Jun 8 20:15:54 AEST 2021


On Tue, 8 Jun 2021, Zoltan Somogyi wrote:

> 2021-06-08 15:21 GMT+10:00 "Julien Fischer" <jfischer at opturion.com>:
>> That test could be stronger, for example:
>>
>>     :- inst key_char for char/0
>>         --->    ('s')
>>         ;       b
>>         ;       0'a
>>         ;       0'Δ
>>         ;       ('\n').
>
> I would not want to add any code using the 0' notation inherited
> from Prolog. Since I wish it would go die in a fire :-(, I don't want to
> even hint that this syntax is something we encourage.

It's not a question of encouraging it, it's a matter of ensuring that
the compiler doesn't do anything untoward with something that it
_currently_ accepts as a character.  You can omit the fourth one, since
to my mind that's questionable; the last one where the is a character
escape definitely needs to be tested (i.e. to check that it is escaped
in the error message).

>> (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.
>
> Those two paragraphs contradict each other :-(
>
> Until someone who knows Unicode very well (maybe Peter, definitely
> not me) has checked the whole mode analysis pass without finding
> any places where we mishandle non-ASCII chars, I would not want
> to make such an announcement. At the moment, I don't think I would
> even want to claim we support insts containing only ASCII chars
> until we have much more experience with this diff ourselves.

Fair enough.  We probably need a proper syntax of character literals
rather than the hodgepodge of things we have now for that to happen.

Julien.


More information about the reviews mailing list