[m-rev.] diff: detect duplicate sub-modules

[Fergus Henderson]

This is still does not completely enforce the rules in 
the Mercury language reference manual, e.g. it allows
more than one `:- include_module' declaration for the
same sub-module.  But it does catch the case which
Pete encountered.

I'm currently running a bootcheck, and will go ahead
and commit this if it passes the tests.

----------

Estimated hours taken: 2
Branches: main

compiler/modules.m:
	Report an error if a sub-module is declared as both
	a nested sub-module and a separate sub-module.

tests/invalid/Mmakefile:
tests/invalid/duplicate_module.m:
tests/invalid/duplicate_module_test.m:
tests/invalid/duplicate_module_test.err_exp:
	A regression test.

Workspace: /home/earth/fjh/ws-earth4/mercury
Index: compiler/modules.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/modules.m,v
retrieving revision 1.200
diff -u -d -r1.200 modules.m
--- compiler/modules.m	6 Nov 2001 15:21:06 -0000	1.200
+++ compiler/modules.m	15 Nov 2001 11:54:55 -0000
@@ -390,8 +390,10 @@
 
 	% Given a module (well, a list of items), split it into
 	% its constituent sub-modules, in top-down order.
-	% Report an error if the `implementation' section of a sub-module
-	% is contained inside the `interface' section of its parent module.
+	% Also do some error checking:
+	% - report an error if the `implementation' section of a sub-module
+	%   is contained inside the `interface' section of its parent module
+	% - check for modules declared as both nested and separate sub-modules.
 
 :- type module_list == list(pair(module_name, item_list)).
 
@@ -4301,17 +4303,9 @@
 	% the previous entry will be a dummy entry that we inserted
 	% after trying to read the source file and failing.
 	%
-	% XXX We could make some effort here to catch the case where a
-	% module is defined as both a separate sub-module and also
-	% as a nested sub-module.  However, that doesn't seem worthwhile,
-	% since not all such cases would arrive here anyway --
-	% it would be nice to catch that case but this is not the
-	% place to catch it.
-	% (Currently for that case we just ignore the file containing the
-	% separate sub-module.  Since we don't consider the
-	% file containing the separate sub-module to be part of the
-	% program's source, there's no duplicate definition, and thus
-	% no requirement to report any error message.)
+	% Note that the case where a module is defined as both a
+	% separate sub-module and also as a nested sub-module is
+	% caught in split_into_submodules.
 	%
 :- pred insert_into_deps_map(module_imports, deps_map, deps_map).
 :- mode insert_into_deps_map(in, in, out) is det.
@@ -5122,7 +5116,19 @@
 	{ InParentInterface = no },
 	split_into_submodules_2(ModuleName, Items0, InParentInterface,
 		Items, ModuleList),
-	{ require(unify(Items, []), "modules.m: items after end_module") }.
+	{ require(unify(Items, []), "modules.m: items after end_module") },
+	%
+	% check for modules declared as both nested and separate sub-modules
+	%
+	{ get_children(Items0, NestedSubmodules) },
+	{ assoc_list__keys(ModuleList, SeparateSubModules) },
+	{ Duplicates = set__intersect(set__list_to_set(NestedSubmodules),
+				set__list_to_set(SeparateSubModules)) },
+	( { set__empty(Duplicates) } ->
+		[]
+	;
+		report_duplicate_modules(Duplicates, Items0)
+	).
 
 :- pred split_into_submodules_2(module_name, item_list, bool, item_list,
 				module_list, io__state, io__state).
@@ -5244,6 +5250,10 @@
 	% Perhaps we should be a bit more strict about this, for
 	% example by only allowing one `:- implementation' section
 	% and one `:- interface' section for each module?
+	% (That is what the Mercury language reference manual mandates.
+	% On the other hand, it also says that top-level modules
+	% should only have one `:- interface' and one `:- implementation'
+	% section, and we don't enforce that either...)
 	%
 	( map__search(SubModules0, ModuleName, ItemList0) ->
 		list__append(ModuleItemList, ItemList0, ItemList),
@@ -5278,6 +5288,50 @@
 	prog_out__write_context(Context),
 	io__write_string(
 		"  occurs in interface section of parent module.\n"),
+	io__set_exit_status(1).
+
+:- pred report_duplicate_modules(set(module_name), item_list,
+		io__state, io__state).
+:- mode report_duplicate_modules(in, in, di, uo) is det.
+
+report_duplicate_modules(Duplicates, Items) -->
+	{ IsDuplicateError = (pred(SubModuleName - Context::out) is nondet :-
+		list__member(Item, Items),
+		Item = module_defn(_VarSet, ModuleDefn) - Context,
+		( ModuleDefn = module(SubModuleName)
+		; ModuleDefn = include_module(SubModuleNames),
+		  list__member(SubModuleName, SubModuleNames)
+		),
+		set__member(SubModuleName, Duplicates)
+	  ) },
+	{ solutions(IsDuplicateError, DuplicateErrors) },
+	list__foldl(report_error_duplicate_module_decl, DuplicateErrors).
+
+:- pred report_error_duplicate_module_decl(pair(module_name, prog_context),
+		io__state, io__state).
+:- mode report_error_duplicate_module_decl(in, di, uo) is det.
+
+report_error_duplicate_module_decl(ModuleName - Context) -->
+	{ ModuleName = qualified(ParentModule0, ChildModule0) ->
+		ParentModule = ParentModule0,
+		ChildModule = ChildModule0
+	;
+		error("report_error_duplicate_module_decl")
+	},
+	% The error message should look this this:
+	% foo.m:123: In module `foo':
+	% foo.m:123:   error: sub-module `bar' declared as both
+	% foo.m:123:   a separate sub-module and a nested sub-module.
+	prog_out__write_context(Context),
+	io__write_string("In module `"),
+	prog_out__write_sym_name(ParentModule),
+	io__write_string("':\n"),
+	prog_out__write_context(Context),
+	io__write_string("  error: sub-module `"),
+	io__write_string(ChildModule),
+	io__write_string("' declared as both\n"),
+	prog_out__write_context(Context),
+	io__write_string("  a separate sub-module and a nested sub-module.\n"),
 	io__set_exit_status(1).
 
 	% Given a module (well, a list of items), extract the interface
Index: tests/invalid/Mmakefile
===================================================================
RCS file: /home/mercury1/repository/tests/invalid/Mmakefile,v
retrieving revision 1.100
diff -u -d -r1.100 Mmakefile
--- tests/invalid/Mmakefile	28 Oct 2001 12:25:58 -0000	1.100
+++ tests/invalid/Mmakefile	15 Nov 2001 12:03:49 -0000
@@ -10,6 +10,9 @@
 # Note: multi-module tests (including tests of nested modules)
 # need to be listed separately from single-module tests, since
 # we need to make dependencies only for multimodule tests.
+# However, multi-module tests where the error is detected when
+# building the dependencies (e.g. duplicate_module_test.m) should 
+# not be included in this list; we handle those specially (see below).
 
 MULTIMODULE_SOURCES= \
 	aditi_errors.m \
@@ -37,6 +40,7 @@
 	constructor_warning.m \
 	det_errors.m \
 	duplicate_modes.m \
+	duplicate_module_test.m \
 	errors.m \
 	errors1.m \
 	errors2.m \
@@ -179,9 +183,10 @@
 MCFLAGS-undef_mod_qual = 	--no-intermodule-optimization
 MCFLAGS-undef_symbol = 		--no-intermodule-optimization
 
-# The bug is caught when generating dependencies, so it is
-# easiest just to do that step.
+# For these test cases, the bug is caught when generating dependencies,
+# so it is easiest just to do that step.
 MCFLAGS-nested_impl_in_int =	--generate-dependencies
+MCFLAGS-duplicate_module_test =	--generate-dependencies
 
 MULTIMODULE_DEPENDS=	$(MULTIMODULE_SOURCES:%.m=%.depend)
 
Index: tests/invalid/duplicate_module.m
===================================================================
RCS file: tests/invalid/duplicate_module.m
diff -N tests/invalid/duplicate_module.m
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ tests/invalid/duplicate_module.m	15 Nov 2001 12:04:26 -0000
@@ -0,0 +1,80 @@
+% Test of modules declared as both nested and separate modules.
+% This is referenced from duplicate_module_test.m.
+
+:- module duplicate_module.
+:- interface.
+:- import_module io.
+
+:- pred do_main(io__state::di, io__state::uo) is det.
+
+:- implementation.
+
+%-----------------------------------------------------------------------------%
+
+:- include_module duplicate_module:child.
+
+:- module duplicate_module:child.
+:- interface.
+:- import_module io.
+
+:- type foo ---> bar ; baz(int).
+
+:- pred hello(io__state::di, io__state::uo) is det.
+
+:- implementation.
+
+hello --> io__write_string("duplicate_module:child:hello\n").
+
+:- end_module duplicate_module:child.
+
+%-----------------------------------------------------------------------------%
+
+:- include_module child2.
+
+:- module duplicate_module:child2.
+:- interface.
+:- import_module io.
+
+:- type foo ---> bar ; baz(int).
+
+:- pred hello(io__state::di, io__state::uo) is det.
+
+:- implementation.
+
+hello --> io__write_string("duplicate_module:child2:hello\n").
+
+:- end_module duplicate_module:child2.
+
+%-----------------------------------------------------------------------------%
+
+:- include_module duplicate_module:child3.
+
+:- module child3.
+:- interface.
+:- import_module io.
+
+:- type foo ---> bar ; baz(int).
+
+:- pred hello(io__state::di, io__state::uo) is det.
+
+:- implementation.
+
+hello --> io__write_string("duplicate_module:child2:hello\n").
+
+:- end_module child3.
+
+%-----------------------------------------------------------------------------%
+
+% now we're back in the parent module.
+
+:- use_module duplicate_module:child.
+:- use_module duplicate_module:child2.
+:- use_module duplicate_module:child3.
+:- import_module std_util, require.
+
+do_main -->
+	duplicate_module:child:hello,
+	duplicate_module:child2:hello,
+	duplicate_module:child3:hello.
+
+:- end_module duplicate_module.
Index: tests/invalid/duplicate_module_test.err_exp
===================================================================
RCS file: tests/invalid/duplicate_module_test.err_exp
diff -N tests/invalid/duplicate_module_test.err_exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ tests/invalid/duplicate_module_test.err_exp	15 Nov 2001 12:08:12 -0000
@@ -0,0 +1,19 @@
+duplicate_module.m:014: In module `duplicate_module':
+duplicate_module.m:014:   error: sub-module `child' declared as both
+duplicate_module.m:014:   a separate sub-module and a nested sub-module.
+duplicate_module.m:016: In module `duplicate_module':
+duplicate_module.m:016:   error: sub-module `child' declared as both
+duplicate_module.m:016:   a separate sub-module and a nested sub-module.
+duplicate_module.m:032: In module `duplicate_module':
+duplicate_module.m:032:   error: sub-module `child2' declared as both
+duplicate_module.m:032:   a separate sub-module and a nested sub-module.
+duplicate_module.m:034: In module `duplicate_module':
+duplicate_module.m:034:   error: sub-module `child2' declared as both
+duplicate_module.m:034:   a separate sub-module and a nested sub-module.
+duplicate_module.m:050: In module `duplicate_module':
+duplicate_module.m:050:   error: sub-module `child3' declared as both
+duplicate_module.m:050:   a separate sub-module and a nested sub-module.
+duplicate_module.m:052: In module `duplicate_module':
+duplicate_module.m:052:   error: sub-module `child3' declared as both
+duplicate_module.m:052:   a separate sub-module and a nested sub-module.
+For more information, try recompiling with `-E'.
Index: tests/invalid/duplicate_module_test.m
===================================================================
RCS file: tests/invalid/duplicate_module_test.m
diff -N tests/invalid/duplicate_module_test.m
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ tests/invalid/duplicate_module_test.m	15 Nov 2001 12:05:16 -0000
@@ -0,0 +1,10 @@
+% Test of modules declared as both nested and separate modules.
+% The error occurs in the imported module duplicate_module.m,
+% but it is detected when making the dependencies for this module.
+:- module duplicate_module_test.
+:- interface.
+:- import_module io.
+:- pred main(io::di, io::uo) is det.
+:- implementation.
+:- import_module duplicate_module.
+main --> duplicate_module__do_main.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------