[m-rev.] for review: fix bug #100

Julien Fischer juliensf at csse.unimelb.edu.au
Tue Feb 2 12:14:21 AEDT 2010


In the absence of a review, I shall commit this.

Julien.

On Wed, 27 Jan 2010, Julien Fischer wrote:

> Fix bug #100.  The compiler is erroneously reporting an import in a module
> interface as unused even though the import is required in order to satisfy 
> the
> superclass constraints on an exported type class instance.  (The situation
> is explained in detail in the diff.)
>
> The fix is to assume that when checking for unused modules, an imported 
> module
> that provides a type class instance is used.  We can (and do) avoid this
> assumption if the importing module does not export any type class instances
> since in that case the erroneous warning cannot occur.
>
> compiler/module_qual.m:
> 	Keep tracking of which imported modules export type class instances
> 	and whether the current module exports any.
>
> 	Using the above information, avoid incorrectly reporting modules
> 	imported in the interface as unused if they are in fact required
> 	because of superclass constraints on instances.
> 	(This is somewhat inelegant, but alternative fixes would require
> 	either (a) delaying the unused module check until after the HLDS
> 	has been constructed, or (b) doing more work on the parse tree
> 	to make information about type class and instances available.
> 	Both of these are much larger changes.)
>
> tests/valid/Mercury.options:
> tests/valid/Mmakefile:
> tests/valid/bug100*.m:
> 	Regression test for bug 100.
>
> tests/warnings/Mmakefile:
> tests/warnings/unused_interface_import*.m:
> tests/warnings/unused_interface_import.exp:
> 	Check that we still emit a warning about unused modules
> 	that export instances if the module compiled does _not_
> 	export any instances.
>
> Index: compiler/module_qual.m
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/compiler/module_qual.m,v
> retrieving revision 1.176
> diff -u -r1.176 module_qual.m
> --- compiler/module_qual.m	5 Nov 2009 06:34:39 -0000	1.176
> +++ compiler/module_qual.m	27 Jan 2010 01:28:24 -0000
> @@ -165,9 +165,43 @@
>     mq_info_get_type_error_flag(!.Info, UndefTypes),
>     mq_info_get_mode_error_flag(!.Info, UndefModes),
>     (
> +        % Warn about any unused module imports in the interface.
> +        % There is a special case involving type class instances that
> +        % we need to handle here.  Consider:
> +        %
> +        %   :- module foo.
> +        %   :- interface.
> +        % +        %   :- import_module bar.
> +        %   :- typeclass tc1(T) <= tc2(T).
> +        %   :- instance tc1(unit).
> +        %
> +        % where module bar exports the instance tc2(unit).  We must import
> +        % the module bar in the interface of the module foo in order for
> +        % the superclass constraint on the instance tc1(unit) to be 
> satisfied.
> +        % However, at this stage of compilation we do not know that the
> +        % instance tc2(unit) needs to be visible.  (Knowing this would 
> require
> +        % a more extensive analysis of type classes and instances to be done
> +        % in this module.)
> +        %
> +        % In order to prevent the import of the module bar being erroneously 
> +        % reported as unused we make the conservative assumption that any
> +        % imported module that exports a type class instance is used in
> +        % the interface of the importing module, except if the importing
> +        % module itself exports _no_ type class instances.
> +        %
>         MaybeFileName = yes(FileName),
>         mq_info_get_unused_interface_modules(!.Info, UnusedImports0),
> -        set.to_sorted_list(UnusedImports0, UnusedImports),
> +        mq_info_get_exported_instances_flag(!.Info, ModuleExportsInstances),
> +        (
> +            ModuleExportsInstances = yes,
> +            mq_info_get_imported_instance_modules(!.Info, InstanceImports),
> +            set.difference(UnusedImports0, InstanceImports, UnusedImports1)
> +        ;
> +            ModuleExportsInstances = no,
> +            UnusedImports1 = UnusedImports0
> +        ),
> +        set.to_sorted_list(UnusedImports1, UnusedImports),
>         maybe_warn_unused_interface_imports(ModuleName, FileName,
>             UnusedImports, !Specs)
>     ;
> @@ -210,6 +244,13 @@
>                 % needed in the interface.
>                 unused_interface_modules    :: set(module_name),
>
> +                % Modules from which `:- instance' declarations have
> +                % been imported.
> +                imported_instance_modules   :: set(module_name),
> +
> +                % Does this module export any type class instances?
> +                exported_instances_flag      :: bool,
> +
>                 % Import status of the current item.
>                 import_status               :: mq_import_status,
>
> @@ -235,6 +276,8 @@
>                 need_qual_flag              :: need_qualifier,
>
>                 maybe_recompilation_info    :: maybe(recompilation_info)
> +
> +
>             ).
>
> :- type partial_qualifier_info
> @@ -361,11 +404,23 @@
>             mq_info_set_classes(Classes, !Info)
>         )
>     ;
> +          Item = item_instance(ItemInstance),
> +          ( mq_info_get_import_status(!.Info, mq_status_imported(_)) ->
> +              InstanceModule = ItemInstance ^ ci_module_containing_instance,
> +              mq_info_get_imported_instance_modules(!.Info,
> +                ImportedInstanceModules0),
> +              set.insert(ImportedInstanceModules0, InstanceModule,
> +                ImportedInstanceModules),
> +              mq_info_set_imported_instance_modules(ImportedInstanceModules,
> +                !Info)
> +          ;
> +              true
> +          )
> +    ;
>         ( Item = item_clause(_)
>         ; Item = item_pred_decl(_)
>         ; Item = item_mode_decl(_)
>         ; Item = item_pragma(_)
> -        ; Item = item_instance(_)
>         ; Item = item_initialise(_)
>         ; Item = item_finalise(_)
>         ; Item = item_mutable(_)
> @@ -845,6 +900,12 @@
>         list.length(Types0, Arity),
>         Id = mq_id(Name0, Arity),
>         mq_info_set_error_context(mqec_instance(Id) - Context, !Info),
> + +        ( mq_info_get_import_status(!.Info, mq_status_exported) ->
> +            mq_info_set_exported_instances_flag(yes, !Info)
> +        ;
> +            true
> +        ),
>
>         % We don't qualify the implementation yet, since that requires
>         % us to resolve overloading.
> @@ -1850,6 +1911,8 @@
>     term.context_init(Context),
>     ErrorContext = mqec_type(mq_id(unqualified(""), 0)) - Context,
>     set.init(InterfaceModules0),
> +    set.init(InstanceModules),
> +    ExportedInstancesFlag = no,
>     get_implicit_dependencies(Items, Globals, ImportDeps, UseDeps),
>     set.list_to_set(ImportDeps `list.append` UseDeps, ImportedModules),
>
> @@ -1869,8 +1932,8 @@
>     ),
>     Info = mq_info(ImportedModules, InterfaceVisibleModules,
>         Empty, Empty, Empty, Empty, Empty, Empty,
> -        InterfaceModules0, mq_status_local, 0,
> -        no, no, ReportErrors, ErrorContext, ModuleName,
> +        InterfaceModules0, InstanceModules, ExportedInstancesFlag,
> +        mq_status_local, 0, no, no, ReportErrors, ErrorContext, ModuleName,
>         may_be_unqualified, MaybeRecompInfo).
>
> :- pred mq_info_get_imported_modules(mq_info::in, set(module_name)::out)
> @@ -1885,6 +1948,9 @@
> :- pred mq_info_get_classes(mq_info::in, class_id_set::out) is det.
> :- pred mq_info_get_unused_interface_modules(mq_info::in,
>     set(module_name)::out) is det.
> +:- pred mq_info_get_imported_instance_modules(mq_info::in,
> +    set(module_name)::out) is det.
> +:- pred mq_info_get_exported_instances_flag(mq_info::in, bool::out) is det.
> :- pred mq_info_get_import_status(mq_info::in, mq_import_status::out) is 
> det.
> % :- pred mq_info_get_type_error_flag(mq_info::in, bool::out) is det.
> % :- pred mq_info_get_mode_error_flag(mq_info::in, bool::out) is det.
> @@ -1900,6 +1966,8 @@
> mq_info_get_modes(Info, Info ^ modes).
> mq_info_get_classes(Info, Info ^ classes).
> mq_info_get_unused_interface_modules(Info, Info ^ unused_interface_modules).
> +mq_info_get_imported_instance_modules(Info, Info ^ 
> imported_instance_modules).
> +mq_info_get_exported_instances_flag(Info, Info ^ exported_instances_flag).
> mq_info_get_import_status(Info, Info ^ import_status).
> mq_info_get_type_error_flag(Info, Info ^ type_error_flag).
> mq_info_get_mode_error_flag(Info, Info ^ mode_error_flag).
> @@ -1926,6 +1994,10 @@
>     mq_info::in, mq_info::out) is det.
> :- pred mq_info_set_unused_interface_modules(set(module_name)::in,
>     mq_info::in, mq_info::out) is det.
> +:- pred mq_info_set_imported_instance_modules(set(module_name)::in,
> +    mq_info::in, mq_info::out) is det.
> +:- pred mq_info_set_exported_instances_flag(bool::in,
> +    mq_info::in, mq_info::out) is det.
> :- pred mq_info_set_import_status(mq_import_status::in,
>     mq_info::in, mq_info::out) is det.
> :- pred mq_info_set_type_error_flag(mq_info::in, mq_info::out) is det.
> @@ -1945,6 +2017,10 @@
> mq_info_set_classes(Classes, Info, Info ^ classes := Classes).
> mq_info_set_unused_interface_modules(Modules, Info,
>     Info ^ unused_interface_modules := Modules).
> +mq_info_set_imported_instance_modules(Modules, Info,
> +    Info ^ imported_instance_modules := Modules).
> +mq_info_set_exported_instances_flag(Flag, Info,
> +    Info ^ exported_instances_flag := Flag).
> mq_info_set_import_status(Status, Info, Info ^ import_status := Status).
> mq_info_set_type_error_flag(Info, Info ^ type_error_flag := yes).
> mq_info_set_mode_error_flag(Info, Info ^ mode_error_flag := yes).
> Index: tests/valid/Mercury.options
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/tests/valid/Mercury.options,v
> retrieving revision 1.62
> diff -u -r1.62 Mercury.options
> --- tests/valid/Mercury.options	18 Nov 2009 23:49:22 -0000	1.62
> +++ tests/valid/Mercury.options	26 Jan 2010 06:51:49 -0000
> @@ -39,6 +39,7 @@
> MCFLAGS-ambig_stress_test   = --type-check-constraints
> MCFLAGS-builtin_false		= --intermodule-optimization
> MCFLAGS-bug85			= -O0 --deforestation
> +MCFLAGS-bug100                  = --halt-at-warn
> MCFLAGS-compl_unify_bug		= -O3
> MCFLAGS-constraint_prop_bug	= -O0 --common-struct 
> --local-constraint-propagation
> MCFLAGS-deforest_bug		= -O3
> Index: tests/valid/Mmakefile
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/tests/valid/Mmakefile,v
> retrieving revision 1.236
> diff -u -r1.236 Mmakefile
> --- tests/valid/Mmakefile	13 Jan 2010 04:37:42 -0000	1.236
> +++ tests/valid/Mmakefile	26 Jan 2010 06:51:26 -0000
> @@ -64,6 +64,7 @@
> 	any_matches_bound \
> 	big_foreign_type \
> 	bug85 \
> +	bug100 \
> 	builtin_false \
> 	call_failure \
> 	common_struct_bug \
> Index: tests/valid/bug100.m
> ===================================================================
> RCS file: tests/valid/bug100.m
> diff -N tests/valid/bug100.m
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ tests/valid/bug100.m	26 Jan 2010 06:50:55 -0000
> @@ -0,0 +1,21 @@
> +% rotd-2010-01-25 and before were incorrectly reporting that the import
> +% of the module bug100_2 in the interface was unused.
> +%
> +:- module bug100.
> +:- interface.
> +
> +:- import_module unit.
> +:- import_module bug100_3.
> +
> +    % We need this import to get that tc2(unit)
> +    % superclasss constraint is satisfied.
> +    %
> +:- import_module bug100_2.
> +
> +:- typeclass tc(T) <= tc2(T) where [].
> +
> +:- instance tc(unit).
> +
> +:- implementation.
> +
> +:- instance tc(unit) where [].
> Index: tests/valid/bug100_2.m
> ===================================================================
> RCS file: tests/valid/bug100_2.m
> diff -N tests/valid/bug100_2.m
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ tests/valid/bug100_2.m	26 Jan 2010 06:50:55 -0000
> @@ -0,0 +1,11 @@
> +:- module bug100_2.
> +:- interface.
> +
> +:- import_module unit.
> +:- import_module bug100_3.
> +
> +:- instance tc2(unit).
> +
> +:- implementation.
> +
> +:- instance tc2(unit) where [].
> Index: tests/valid/bug100_3.m
> ===================================================================
> RCS file: tests/valid/bug100_3.m
> diff -N tests/valid/bug100_3.m
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ tests/valid/bug100_3.m	26 Jan 2010 06:50:55 -0000
> @@ -0,0 +1,6 @@
> +:- module bug100_3.
> +:- interface.
> +
> +:- typeclass tc2(T) where [].
> +
> +:- implementation.
> Index: tests/warnings/Mmakefile
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/tests/warnings/Mmakefile,v
> retrieving revision 1.48
> diff -u -r1.48 Mmakefile
> --- tests/warnings/Mmakefile	8 Sep 2009 08:14:42 -0000	1.48
> +++ tests/warnings/Mmakefile	27 Jan 2010 01:07:14 -0000
> @@ -34,6 +34,7 @@
> 	unify_f_g \
> 	unused_args_test \
> 	unused_import \
> +	unused_interface_import \
> 	warn_contiguous_foreign \
> 	warn_non_contiguous \
> 	warn_non_contiguous_foreign \
> Index: tests/warnings/unused_interface_import.m
> ===================================================================
> RCS file: tests/warnings/unused_interface_import.m
> diff -N tests/warnings/unused_interface_import.m
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ tests/warnings/unused_interface_import.m	27 Jan 2010 01:31:53 -0000
> @@ -0,0 +1,13 @@
> +:- module unused_interface_import.
> +:- interface.
> +
> +:- import_module unused_interface_import3.
> +:- import_module unused_interface_import2.	% Should warn about this.
> +
> +:- typeclass tc1(T) <= tc2(T) where [].
> +
> +:- implementation.
> +
> +:- import_module unit.
> +
> +:- instance tc1(unit) where [].
> Index: tests/warnings/unused_interface_import2.m
> ===================================================================
> RCS file: tests/warnings/unused_interface_import2.m
> diff -N tests/warnings/unused_interface_import2.m
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ tests/warnings/unused_interface_import2.m	27 Jan 2010 01:06:38 -0000
> @@ -0,0 +1,12 @@
> +:- module unused_interface_import2.
> +:- interface.
> +
> +:- import_module unit.
> +
> +:- import_module unused_interface_import3.
> +
> +:- instance tc2(unit).
> +
> +:- implementation.
> +
> +:- instance tc2(unit) where [].
> Index: tests/warnings/unused_interface_import3.m
> ===================================================================
> RCS file: tests/warnings/unused_interface_import3.m
> diff -N tests/warnings/unused_interface_import3.m
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ tests/warnings/unused_interface_import3.m	27 Jan 2010 01:06:38 -0000
> @@ -0,0 +1,5 @@
> +:- module unused_interface_import3.
> +:- interface.
> +
> +:- typeclass tc2(T) where [].
> +
>
> --------------------------------------------------------------------------
> mercury-reviews mailing list
> Post messages to:       mercury-reviews at csse.unimelb.edu.au
> Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
> Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
> --------------------------------------------------------------------------
>
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------



More information about the reviews mailing list