[m-rev.] for post-commit review: document requirement for non-abstract instances in interfaces
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
> *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
>> 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
> If you are trying to export this instance, move this instance
> declaration to the implementation section ...
>> + 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.
More information about the reviews