[m-rev.] for review: prepare for two new warnings on instances

[Julien Fischer]

On Sat, 19 Mar 2022, Zoltan Somogyi wrote:

>
> 2022-03-19 13:58 GMT+11:00 "Julien Fischer" <jfischer at opturion.com>:
>>> +        "--warn-unnecessarily-private-instance",
>>> +        "\tGenerate a warning if an instance declaration is not exported",
>>> +        "\teven though none of the type constructors mentioned in the",
>>> +        % XXX Should this be like this (what Julien proposed)
>>> +        "\tmember types are defined in the current module.",
>>> +        % XXX or like this (my preference)?
>>> +        "\tmember types are local to the current module.",
>>> +        "--warn-inconsistent-instance-order",
>>> +        "\tGenerate a warning if the order of concrete instance declarations",
>>> +        "\tdoes not match the order of the corresponding abstract",
>>> +        "\tdeclarations.",
>>
>> What I said on the mailing list was "non-local" types, which is a bit
>> ambiguous; I probably should have said was "non-private".
>> You should get the warning if a module has a non-exported type class instance
>> whose type vector consists only of (a) imported types** or (b) types exported
>> in the interface of the _current_ module.
>
> Then I think we agree, but I don't think that description is clear.
>
> How about
>
> "Generate a warning if an instance declaration is not exported
> even though none of the type constructors mentioned in the member types
> is private to the current module."?
>
>> Should we get the warning if there are any class constraints on the
>> instance that refer to private type classes?
>
> No.
>
> We could go with
>
> "Generate a warning if an instance declaration is not exported
> even though none of the type constructors mentioned in the member types
> is private to the current module, and none of its constraints refer
> to private type classes".
>
> Or we could switch to another approach, which clarifies the intent:
>
> "An instance declaration has to be private if some of its member types
> refer to type constructors private to the current module, or if its constraints
> refer to type classes that are private to the current module. Generate
> a warning if an instance declaration is not exported even though
> neither of these conditions apply."
>
> Overall, I think I prefer that last approach. We could even expand
> on it, e.g. by putting "Such instances can only be relevant in the current module."
> between the two sentences above, then possibly replacing the last sentence
> with "Generate a warning if an instance declaration that can be relevant outside
> the current module is not exported.".

I think that last approach is clearest.

>> ** Types imported from private submodules are not non-private so should not
>> trigger the warning.
>
> Want to rephrase that one without the triply nested negation? :-)

Hmmm, my last edit on that sentence didn't too well.

> Are you trying to say that we should replace "private to the current module"
> with something like "private to the current module and its submodules (if any)"?

Yes, although only to its private submodules (i.e. those where the
include_module declaration is in the implementation of the parent.)

Julien.