[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