[m-rev.] for review: fix mantis bug 412

[Peter Wang]

On Thu, 14 Jul 2016 20:45:02 +0200 (CEST), "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> For review by Peter, since he filed the bug.

> Fix a bug in handling instances with --warn-unused-imports.
> 
> This fixes Mantis bug #412.
> 
> compiler/unused_imports.m:
>     Consider that an instance declaration makes a module "used" only if
>     the typeclass involved is itself defined in module that is known to

defined in a module

>     be used for something specific. Mantis bug 412 was caused by instance
>     declarations for classes whose defining modules were imported *implicitly*,
>     and weren't actually used.
> 
>     Improve the infrastructure for debugging similar problems.
> 
>     Note some possibilities for future improvement.
> 
> tests/warnings/bug412.{m,exp}:
>     The test case for this bug.
> 
> tests/warnings/Mmakefile:
> tests/warnings/Mercury.options:
>     Enable the new test case.
> 
> tests/invalid/import_in_parent.err_exp:
>     Update the expected output for this test case. The parent module
>     does not use the imported module (bool) at all, so this is what the
>     error message says after this diff, though its submodule does use bool.


> diff --git a/compiler/unused_imports.m b/compiler/unused_imports.m
> index 430085d..c5f9434 100644
> --- a/compiler/unused_imports.m
> +++ b/compiler/unused_imports.m
...
> +
> +    % If a class is defined in a module that isn't used by any of the HLDS
> +    % entities that we process before instances (which is *all* entities
> +    % other than class instances themselves), then even if e.g. module mx
> +    % defines an instance of such a class, it shouldn't make module mx "used".

This description confuses the modules:

    If a class is defined in a module ClassModule that isn't used by any
    of the HLS entities ... , then even if e.g. InstanceModule defines an
    instance of such a class, it shouldn't make InstanceModule "used".

But it should be ClassModule that is used or not, as in:

> +        record_sym_name_module_as_used(Visibility, ClassName, !UsedModules),
>          list.foldl(mer_type_used_modules(Visibility), Types, !UsedModules),


The proposed change will cause the compiler to warn about a module
being unused if it is imported ONLY for a class definition, even if it
is necessarily imported by the current module in order to define an
instance of that class.

Perhaps a fix is along these lines: only instances defined in "this"
module should "use" the modules defining the class and types in the
instance.  Instances defined in imported modules would be ignored.

Peter
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: mx.m
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20160716/6be24df4/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: my.m
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20160716/6be24df4/attachment-0001.ksh>