[m-rev.] for tryout: type_spec_constrained_preds

[Zoltan Somogyi]

On 2024-02-02 01:49 +11:00 AEDT, "Julien Fischer" <jfischer at opturion.com> wrote:
> For (1) we can add the following pragma:
> 
> :- pragma type_spec_constrained_preds(
>         [stream.line_oriented(Stream, State),
>             stream.unboxed_reader(Stream, char, State, Error),
>             stream.putback(Stream, char, State, Error)],
>         apply_to_superclasses,
>         [
>             subst([Stream = string_reader,
>             State = string_reader_state,
>             Error = string_reader_error])]).
> 
> That compiles successfully at the standard optimisation level;

That itself seems to me to be a problem. The reason why I think that
is that I manually added a type_spec pragma with the above substitution
for the three predicates listed in the error message for the intermodule case:
make_error_context, make_syntax_error, and make_unexpected_eof_error.
It worked for the last two, but gave an error for the first. That error is
that make_error_context does not have a type var named Error occurring
in the types of its args, BUT it does have a type var named Error occurring
in its typeclass constraints. The fourth sentence of section 11.5 of the reference
manual (rotd version) says that this should not happen, but it seems the compiler
does not check compliance with this rule *unless* you happen to have a type_spec
pragma for the predicate :-(

That seems to me a preexisting bug, but probably not the kind that you were
expecting.

I don't know why you didn't see the error message I saw.
The code handling the new pragma, when it invokes the existing code
to add the type_spec pragmas it generates to the HLDS, is intended
to eventually throw away any resulting error_specs, and had a comment
to this effect, but the actual code still kept the error_specs, so they
should have been printed.

> with
> --intermodule-optimization enabled, we trip over what I suspect is
> an existing bug:

It is actually at least two preexisting bugs.

One bug, which I have fixed, is the absence of contexts for the error messages.

Another problem is that the definition make_error_context is not
written to the .opt file, probably due to the presence of the promise_pure scope
in it. This may be a NYI rather than a bug.

The second definite bug is that the compiler, for some not-yet-known reason,
interprets the presence of a declaration for make_error_context but no definition,
or maybe something else, as cause to believe that it has TWO modes, rather than one.
(At any rate, its proc table has entries for proc ids 0 and 1.) When it processes
the type_spec pragma for this predicate, whether that pragma is hand-written
or compiler-generated, it records that this pragma is applicable to procs 0 and 1.
During the post-typecheck pass, one procedure is deleted; I don't yet know why.
But this confuses the purity checking algorithm. When it checks the purity
of a clause that is specified to be applicable to a set of modes (i.e. procs),
it considers that clause to make its predicate impure unless the set of modes
to which the clause is applicable is *equal* to the set of modes of the predicate.
In this case, the clause is applicable to *all* modes of the predicate, but it says
it is also applicable to *more* modes, so this test fails. In the absence of a
promise_equivalent_causes pragma, the purity pass considers make_error_context
to be impure, and since the definition of its type specialized version is just a call
to make_error_context (that higher_order.m is supposed to inline and specialize later),
it considers that version to be impure as well. Hence the error messages.

I have a workaround, which is to get the code implementing the old type_spec
pragmas to use mode-specific clauses only necessary. This avoids the problem
in this case, and in all cases resulting from the use of type_spec_constrained_preds
pragmas, since these always result in the generation of type_spec pragmas are
NOT mode specific.

> For (2) and (3) we can add the following pragmas:
> 
> :- pragma type_spec_constrained_preds(
>         [
>          stream.writer(Stream, string, State)
>         ],
>         apply_to_superclasses,
>         [subst([
>            Stream = io.text_output_stream,
>            State = io.state])]).
> 
> :- pragma type_spec_constrained_preds(
>         [
>          stream.writer(Stream, char, State),
>          stream.writer(Stream, string, State)
>         ],
>         apply_to_superclasses,
>         [subst([
>             Stream = string.builder.handle,
>             State = string.builder.state])]).
> 
> Each of these works by itself, but both of them being present results in
> the following abort during parse tree -> HLDS conversion.
> 
>    Uncaught Mercury exception:
>    Software Error: predicate `one_or_more.det_list_to_one_or_more'/2:
>    Unexpected: empty list
>    Stack dump not available in this grade.
>    ** Error making
>    `Mercury/asm_fast.gc/x86_64-pc-linux-gnu/Mercury/opts/json.writer.opt'.

This was trivial to fix. We talked earlier about type_spec pragmas not being
composable; this bug was caused by their implementation not being set up
for composability. It occurred when we tried to apply a type_spec pragma
to a predicate we created as part of the implementation of an *earlier*
type_spec pragma. The fix is simple: don't try to apply one type_spec
pragma to the output of another. If we decide we want composability,
it will be done a different commit.

> In the cases that succeed, the specialisations I would expect are
> present in the generated C code, although I have not looked at how
> effective they are.

The diff I committed earlier today should help with that.

I am attaching the three files that I have modified as part of the fixes
described above. If you apply them to the earlier version, you should get
my current version. Can you please check whether there are any remaining
problems, either with csv or json?

Zoltan.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: add_pragma_type_spec.m
Type: application/octet-stream
Size: 72494 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20240202/7c86d885/attachment-0003.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: hlds_out_pred.m
Type: application/octet-stream
Size: 54464 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20240202/7c86d885/attachment-0004.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: maybe_error.m
Type: application/octet-stream
Size: 5949 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20240202/7c86d885/attachment-0005.obj>