[m-rev.] for post-commit review: document requirement for non-abstract instances in interfaces

Zoltan Somogyi zoltan.somogyi at runbox.com
Thu Feb 3 12:41:26 AEDT 2022


2022-02-03 12:33 GMT+11:00 "Julien Fischer" <jfischer at opturion.com>:
>> Julien, you added the code to the compiler to enforce this requirement
>> in April 2005. The log message does not give the reason for the requirement.
>> The m-rev thread on that diff says that the requirement is in the reference manual,
>> but I have found no trace of that. If you happen to remember the reason
>> for this design decision, please modify reference_manual.texi to mention it.
> 
> That's almost 17 years ago!  (There was almost certainly some in-office
> discussion regarding this that isn't captured by the mailing list.)

The first draft of that sentence started as "In the unlikely event that you
remember ..."

> I
> *think* the main reason for that design decision was that allowing
> non-abstract instance declarations in module interfaces effectively
> allows clauses (via inline method definitions) in module interfaces.

Ok, that is something we can give in the manual and expect it to be
understood.

>>  report_non_abstract_instance_in_interface(Context, !Specs) :-
>> -    Pieces = [words("Error: non-abstract instance declaration"),
>> +    AlwaysPieces = [words("Error: non-abstract instance declaration"),
>>          words("in module interface."), nl],
>> -    Spec = simplest_spec($pred, severity_error, phase_parse_tree_to_hlds,
>> -        Context, Pieces),
>> +    VerbosePieces = [words("The usual fix for this problem"),
> 
> Why "usual" fix?  If the intention is that the instance be exported,
> it's the only fix.

Agreed. I meant "in the usual case where the instance is in the interface
section because you want to export it".

> Maybe it would better to start that with something
> like:
> 
>      If you are trying to export this instance, move this instance
>      declaration to the implementation section ...

Will do.
 
>> +        words("is to move this instance declaration"),
>> +        words("to the implementation section,"),
>> +        words("replacing it in the interface section"),
>> +        words("with its abstract version, which contains"),
>> +        words("just the class name and the instance type vector."), nl],
> 
> In the case where there are type class constraints on the instance declaration,
> they also need to be replicated in the abstract declaration.

I will reword what I added to take that into account, but section 10.4 does NOT
mention this point (neither does it deny it), and does not give an example
either. After I make the changes mentioned above, you may want to fix that.

Zoltan.


More information about the reviews mailing list