[m-rev.] for review: check bound insts in coerce ops more systematically

[Zoltan Somogyi]

On 2024-05-30 15:53 +10:00 AEST, "Peter Wang" <novalazy at gmail.com> wrote:
>> +    ;       bad_cons_id_input_inst_arity(cons_id, arity, arity)
>> +            % The cons_id exists in the input type, but the inst specifies
>> +            % the wrong arity for it. XXX How?
>> +            % The cons_id, its arity in the inst of X, and the expected arity.
> 
> I wasn't able to (quickly) construct a test case which triggers this
> error.

I am not surprised.
 
> I may have just have put that arity check in
> modecheck_coerce_from_bound_make_bound_functor out of caution,
> since as the comment says:
> 
>     % The user may have provided an inst that does not make sense for the type.
>     % The compiler does not check for that elsewhere (probably it should)
>     % so we try to be careful about that here.

I think that caution is warranted. I have updated that comment to this:

% The user may have provided an inst that does not make sense for the type.
% The compiler *should* check for that elsewhere, in types_into_modes.m,
% but neither wangp nor I (zs) are confident that it guarantees catching
% all such errors (mostly because it may not be *invoked* on every inst
% that should be checked).

> You may want to merge bad_cons_id_input_inst_arity into
> bad_cons_id_input.

Now that the code is written, I don't think that would gain anything.

>> -        BadConsIds = [_ | _],
>> +        BadConsIdErrors = [HeadBadConsIdError | TailBadConsIdErrors],
>>          % This will prefer to report any invalid functors in the current
>>          % `bound()' inst over an error found deeper in the inst tree
>>          % (in MaybeError0).
> 
> Update or delete this comment (MaybeError0 to DeeperErrors).

I have updated it to

% We report both
% - the invalid functors in BoundInstsX (ConsIdError), and
% - any errors found deeper in the inst tree (DeeperErrors).

> That looks fine.

Thanks. I followed all your other suggestions as well.

Zoltan.