[m-rev.] for review: warn about insts without matching types

[Ian MacLarty]

On Fri, Jul 14, 2006 at 04:40:38PM +1000, Julien Fischer wrote:
> 
> On Fri, 14 Jul 2006, Ian MacLarty wrote:
> 
> >>Also, we (currently) need to support the situation where an inst in the
> >>interface refers to constructors in the implementation (due to the
> >>implementation not supporting abstract insts), e.g.
> >>
> >>	:- interface.
> >>
> >>	:- type fruit.
> >>	:- type citrus ---> lemon ; orange.
> >
> >You mean ":- inst citrus ...".
> 
> Yes, I did mean inst there.
> 
> That (pending your bootcheck) looks fine.
> 

Bootcheck failed (sorry, I should have waited for it to finish before
posting).  There were two problems with my last change:

1. I ignored opt_imported types when looking for matching types
   for an inst, however imported abstract types have status abstract_imported.
   To fix this I've removed hlds_pred.status_is_user_visible/2 and added
   inst_check.status_implies_type_defn_is_user_visible/2.

2. In my test case the inst that I was using to check that it didn't
   match the abstract type t/0 had the function symbol t/0 which matches
   the character builtin type, so I didn't get the expected warning.

Here is the (bootchecked) interdiff:

reverted:
--- compiler/hlds_pred.m	14 Jul 2006 05:32:29 -0000
+++ compiler/hlds_pred.m	12 Jul 2006 02:51:00 -0000	1.200
@@ -269,12 +269,6 @@
     %
 :- pred status_is_imported(import_status::in, bool::out) is det.
 
-    % Returns yes if the item is user visible in the current module,
-    % i.e. that the item is defined locally or is imported, but not
-    % opt-imported.
-    %
-:- pred status_is_user_visible(import_status::in, bool::out) is det.
-
     % Returns yes if the status indicates that the item was
     % defined in this module.  This is the opposite of
     % status_is_imported.
@@ -864,18 +858,6 @@
 status_defined_in_this_module(exported_to_submodules,   yes).
 status_defined_in_this_module(local,                    yes).
 
-status_is_user_visible(imported(_),                     yes).
-status_is_user_visible(external(_),                     yes).
-status_is_user_visible(abstract_imported,               yes).
-status_is_user_visible(pseudo_imported,                 yes).
-status_is_user_visible(opt_imported,                    no).
-status_is_user_visible(exported,                        yes).
-status_is_user_visible(opt_exported,                    yes).
-status_is_user_visible(abstract_exported,               yes).
-status_is_user_visible(pseudo_exported,                 yes).
-status_is_user_visible(exported_to_submodules,          yes).
-status_is_user_visible(local,                           yes).
-
 calls_are_fully_qualified(Markers) =
     ( check_marker(Markers, calls_are_fully_qualified) ->
         is_fully_qualified
diff -u compiler/inst_check.m compiler/inst_check.m
--- compiler/inst_check.m	14 Jul 2006 05:32:29 -0000
+++ compiler/inst_check.m	17 Jul 2006 07:25:30 -0000
@@ -74,6 +74,24 @@
         assoc_list.keys(InstIdDefPairsForCurrentModule), 
         assoc_list.values(InstIdDefPairsForCurrentModule), !IO).
 
+    % Returns yes if a type definition with the given import status
+    % is user visible in the current module.
+    %
+:- pred status_implies_type_defn_is_user_visible(import_status::in, bool::out)
+    is det.
+
+status_implies_type_defn_is_user_visible(imported(_),               yes).
+status_implies_type_defn_is_user_visible(external(_),               no).
+status_implies_type_defn_is_user_visible(abstract_imported,         no).
+status_implies_type_defn_is_user_visible(pseudo_imported,           no).
+status_implies_type_defn_is_user_visible(opt_imported,              no).
+status_implies_type_defn_is_user_visible(exported,                  yes).
+status_implies_type_defn_is_user_visible(opt_exported,              yes).
+status_implies_type_defn_is_user_visible(abstract_exported,         yes).
+status_implies_type_defn_is_user_visible(pseudo_exported,           yes).
+status_implies_type_defn_is_user_visible(exported_to_submodules,    yes).
+status_implies_type_defn_is_user_visible(local,                     yes).
+
 :- pred inst_is_defined_in_current_module(pair(inst_id, hlds_inst_defn)::in)
     is semidet.
 
@@ -85,7 +103,7 @@
 
 type_is_user_visible(TypeDef) :-
     get_type_defn_status(TypeDef, ImportStatus),
-    status_is_user_visible(ImportStatus, yes).
+    status_implies_type_defn_is_user_visible(ImportStatus, yes).
 
 :- type functors_to_types == multi_map(sym_name_and_arity, hlds_type_defn).
 
diff -u tests/warnings/inst_with_no_type.exp tests/warnings/inst_with_no_type.exp
--- tests/warnings/inst_with_no_type.exp	14 Jul 2006 05:49:49 -0000
+++ tests/warnings/inst_with_no_type.exp	18 Jul 2006 07:44:56 -0000
@@ -12,4 +12,4 @@
+inst_with_no_type.m:092: Warning: inst `inst_with_no_type.t_no_match'/0 does
+inst_with_no_type.m:092:   not match any of the types in scope.
 inst_with_no_type.m:076: Warning: inst `inst_with_no_type.unique_no_match'/0
 inst_with_no_type.m:076:   does not match any of the types in scope.
-inst_with_no_type.m:092: Warning: inst `inst_with_no_type.t_no_match'/0
-inst_with_no_type.m:092:   does not match any of the types in scope.
diff -u tests/warnings/inst_with_no_type.m tests/warnings/inst_with_no_type.m
--- tests/warnings/inst_with_no_type.m	14 Jul 2006 05:32:29 -0000
+++ tests/warnings/inst_with_no_type.m	18 Jul 2006 07:42:26 -0000
@@ -89,7 +89,7 @@
 	% insts, since the function symbol t is not user visible from 
 	% this scope.
 	%
-:- inst t_no_match ---> t.
+:- inst t_no_match ---> ft.
 
 :- type fruit ---> apple ; orange ; lemon.
 
diff -u tests/warnings/inst_with_no_type_2.m tests/warnings/inst_with_no_type_2.m
--- tests/warnings/inst_with_no_type_2.m	14 Jul 2006 05:33:32 -0000
+++ tests/warnings/inst_with_no_type_2.m	18 Jul 2006 07:42:43 -0000
@@ -12,4 +12,5 @@
 
-:- type t ---> t.
+:- type t ---> ft.
 
+:- pragma inline(p/3).
 p(T, !IO) :- write(T, !IO).
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at csse.unimelb.edu.au
administrative address: owner-mercury-reviews at csse.unimelb.edu.au
unsubscribe: Address: mercury-reviews-request at csse.unimelb.edu.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at csse.unimelb.edu.au Message: subscribe
--------------------------------------------------------------------------