[m-rev.] diff: improve instance consistency check

[Julien Fischer]

Estimated hours taken: 1
Branches: main

Improve the consistency check for type class instances.  In particular,
inconsistent instances in parent and child modules will now be detected
without --intermodule-optimization being enabled.

compiler/check_typeclass.m:
 	If --intermodule-optimization is not enabled then don't filter
 	out imported abstract instances before doing the consistency check.
 	(If --intermodule-optimization *is* enabled then we don't need
 	the abstract instances because we will have the concrete ones for
 	the .opt files.)

tests/invalid/Mmakefile:
tests/invalid/Mercury.options:
tests/invalid/ii_parent.m:
tests/invalid/ii_parent.ii_child.m:
tests/invalid/ii_parent.ii_child.err_exp:
 	Test the above inconsistent instances in parent and child modules
 	are detected when --intermodule-optimzation is not enabled.

tests/invalid/sub_c.m:
 	Unrelated change: update the syntax for module qualifiers.

Julien.


Index: compiler/check_typeclass.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/check_typeclass.m,v
retrieving revision 1.122
diff -u -r1.122 check_typeclass.m
--- compiler/check_typeclass.m	21 Jul 2008 03:10:06 -0000	1.122
+++ compiler/check_typeclass.m	5 Apr 2009 12:05:22 -0000
@@ -70,12 +70,7 @@
  % dependencies.  This doesn't necessarily catch all cases of inconsistent
  % instances, however, since in general that cannot be done until link time.
  % We try to catch as many cases as possible here, though, since we can give
-% better error messages.  Note that we only check concrete instance definitions
-% for mutual consistency, since otherwise the abstract instance definitions
-% would conflict with their corresponding concrete definitions.  If in
-% future we allow stub instances, where the concrete definition is compiler
-% generated, we should add the concrete definitions before this pass to
-% ensure that they get checked.
+% better error messages.
  %
  % (6) in check_typeclass_constraints/4, we check typeclass constraints on
  % predicate and function declarations and on existentially typed data
@@ -1142,10 +1137,24 @@
      map.lookup(InstanceTable, ClassId, InstanceDefns),
      FunDeps = ClassDefn ^ class_fundeps,
      check_coverage(ClassId, InstanceDefns, FunDeps, !ModuleInfo, !Specs),
-    % Concrete definitions will always overlap with abstract ones; we only
-    % need to check the former.
-    list.filter(is_concrete_instance_defn, InstanceDefns,
-        ConcreteInstanceDefns),
+    module_info_get_globals(!.ModuleInfo, Globals),
+    % Abstract definitions will always overlap with concrete definitions,
+    % so we filter out the abstract definitions for this module.  If
+    % --intermodule-optimization is enabled then we strip out the imported
+    % abstract definitions for all modules since we will have the concrete
+    % definitions for imported instances from the .opt files.  If it is not
+    % enabled then we keep the abstract definitions for imported instances
+    % since doing so may allow us to detect errors.
+    globals.lookup_bool_option(Globals, intermodule_optimization, IntermodOpt),
+    (
+        IntermodOpt = yes,
+        list.filter(is_concrete_instance_defn, InstanceDefns,
+            ConcreteInstanceDefns)
+    ;
+        IntermodOpt = no,
+        list.filter(is_concrete_or_imported_instance_defn, InstanceDefns,
+            ConcreteInstanceDefns)
+    ),
      check_consistency(ClassId, ClassDefn, ConcreteInstanceDefns, FunDeps,
          !ModuleInfo, !Specs).

@@ -1154,6 +1163,16 @@
  is_concrete_instance_defn(InstanceDefn) :-
      InstanceDefn ^ instance_body = instance_body_concrete(_).

+:- pred is_concrete_or_imported_instance_defn(hlds_instance_defn::in)
+    is semidet.
+
+is_concrete_or_imported_instance_defn(InstanceDefn) :-
+    (
+        is_concrete_instance_defn(InstanceDefn)
+    ;
+        status_is_imported(InstanceDefn ^ instance_status) = yes
+    ).
+
  :- pred check_coverage(class_id::in, list(hlds_instance_defn)::in,
      hlds_class_fundeps::in, module_info::in, module_info::out,
      list(error_spec)::in, list(error_spec)::out) is det.
Index: tests/invalid/Mercury.options
===================================================================
RCS file: /home/mercury/mercury1/repository/tests/invalid/Mercury.options,v
retrieving revision 1.30
diff -u -r1.30 Mercury.options
--- tests/invalid/Mercury.options	9 Sep 2008 05:26:04 -0000	1.30
+++ tests/invalid/Mercury.options	3 Apr 2009 19:38:00 -0000
@@ -35,6 +35,10 @@
  				--no-automatic-intermodule-optimization
  MCFLAGS-foreign_type_visibility = --no-intermodule-optimization \
  				--no-automatic-intermodule-optimization
+MCFLAGS-ii_parent = 		--no-intermodule-optimization \
+				--no-automatic-intermodule-optimization
+MCFLAGS-ii_parent.ii_child =	--no-intermodule-optimization \
+				--no-automatic-intermodule-optimization
  MCFLAGS-illtyped_compare =	--no-intermodule-optimization \
  				--no-automatic-intermodule-optimization \
  				--verbose-error-messages
Index: tests/invalid/Mmakefile
===================================================================
RCS file: /home/mercury/mercury1/repository/tests/invalid/Mmakefile,v
retrieving revision 1.233
diff -u -r1.233 Mmakefile
--- tests/invalid/Mmakefile	10 Mar 2009 05:00:34 -0000	1.233
+++ tests/invalid/Mmakefile	5 Apr 2009 11:23:10 -0000
@@ -13,6 +13,7 @@
  	duplicate_instance_2 \
  	exported_unify \
  	exported_unify3 \
+	ii_parent.ii_child \
  	ho_default_func_2.sub \
  	import_in_parent \
  	imported_mode \
Index: tests/invalid/ii_parent.ii_child.err_exp
===================================================================
RCS file: tests/invalid/ii_parent.ii_child.err_exp
diff -N tests/invalid/ii_parent.ii_child.err_exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ tests/invalid/ii_parent.ii_child.err_exp	5 Apr 2009 11:21:19 -0000
@@ -0,0 +1,6 @@
+ii_parent.int0:006: Inconsistent instance declaration for typeclass
+ii_parent.int0:006:   `ii_parent.foo'/2 with functional dependency `(A -> B)'.
+ii_parent.ii_child.m:008: Here is the conflicting instance.
+ii_parent.int0:008: Inconsistent instance declaration for typeclass
+ii_parent.int0:008:   `ii_parent.foo'/2 with functional dependency `(A -> B)'.
+ii_parent.ii_child.m:008: Here is the conflicting instance.
Index: tests/invalid/ii_parent.ii_child.m
===================================================================
RCS file: tests/invalid/ii_parent.ii_child.m
diff -N tests/invalid/ii_parent.ii_child.m
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ tests/invalid/ii_parent.ii_child.m	5 Apr 2009 11:17:16 -0000
@@ -0,0 +1,8 @@
+:- module ii_parent.ii_child.
+:- interface.
+
+:- instance foo(int, string).
+
+:- implementation.
+
+:- instance foo(int, string) where [].
Index: tests/invalid/ii_parent.m
===================================================================
RCS file: tests/invalid/ii_parent.m
diff -N tests/invalid/ii_parent.m
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ tests/invalid/ii_parent.m	3 Apr 2009 20:28:10 -0000
@@ -0,0 +1,18 @@
+% rotd-2009-04-03 and before failed to detect the overlapping
+% instance in the child module unless --intermodule-optimization
+% was enabled, despite the fact that the necessary information
+% for doing is contained in the parent module's private
+% interface.
+%
+:- module ii_parent.
+:- interface.
+
+:- typeclass foo(A, B) <= (A -> B) where [].
+
+:- instance foo(int, float).
+
+:- include_module ii_child.
+
+:- implementation.
+
+:- instance foo(int, float) where [].
Index: tests/invalid/sub_c.m
===================================================================
RCS file: /home/mercury/mercury1/repository/tests/invalid/sub_c.m,v
retrieving revision 1.1
diff -u -r1.1 sub_c.m
--- tests/invalid/sub_c.m	24 Sep 1998 19:11:42 -0000	1.1
+++ tests/invalid/sub_c.m	3 Apr 2009 20:38:52 -0000
@@ -9,7 +9,7 @@

  :- implementation.

-:- import_module sub_a:sub1.
+:- import_module sub_a.sub1.

  main -->
          io__write_string("Hello.\n").

--------------------------------------------------------------------------
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
--------------------------------------------------------------------------