[m-rev.] for review: fix bug #100
Julien Fischer
juliensf at csse.unimelb.edu.au
Wed Jan 27 13:37:39 AEDT 2010
For review by anyone.
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.
Julien.
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
--------------------------------------------------------------------------
More information about the reviews
mailing list