[m-rev.] for review: report errors for bad ":- end_module" declarations

[Julien Fischer]

Hi Zoltan,

On Mon, 1 Sep 2014, Zoltan Somogyi wrote:

> Report errors for bad :- end_module declarations.
> 
> Until this diff, the compiler used to crash on such inputs.

Once this is committed you can mark bug #41 in Mantis as resolved.

> compiler/prog_io.m:
>     When reading in :- module and :- end_module items, we used to blindly
>     believe what they said. We now check whether they make sense, and report a
>     fatal error if they don't. Specifically,
>
>     - if the name of the module started by a :- module declaration is
>       qualified, that qualification has to match the name of the
>       previously-current module, and
>
>     - the name of the module ended by a :- end_module declaration
>       has to match the name of the previously-current module.
>
>     Simplify the code we used to use to remove the :- module/:- end_module
>     wrappers around the top level module. It seems to be the loosening
>     of the checks in this code, more than a decade ago, to account for
>     the possibility of the final :- end_module in a file's item list
>     being for a submodule of the main module, not the main module itself,
>     that I think previously let the malformed item list through to be
>     processed by later compiler passes, which did not expect it.

That last sentence is very awkward.

>     Simplify the code for reading in the whole item list. Previously
>     this was done by two mutually-recursive predicates, one of which
>     read in a term for the next item and one which had the term handed
>     to it by its caller. We now handle the two cases (one of which is
>     needed only by the special handling required by the first item)
>     in one self-recursive predicate with a maybe argument.
> 
> mdbcomp/prim_data.m:
>     Add a utility predicate for use by the new code in prog_io.m.
>
>     Remove a comment on code that duplicates the code on the declaration.
> 
> compiler/modules.m:
>     Improve some code that handles lists of items.
>
>     - Make some previously not-tail-recursive predicates tail recursive.
>     - Give some predicates and variables more meaningful names.
>     - Combine some checks where this allows us to reduce the number of
>       traversals over the item list.
> 
> doc/reference_manual.texi:
>     Fix bad punctuation, and expand the list of things
>     that can be declared but defined.

that can be declared but defined?  that can be declared and must be defined?

> tests/invalid/bad_end_module.{m,err_exp}:
> tests/invalid/bad_start_end_module.{m,err_exp}:
>     Test the expected output.
> 
> tests/invalid/Mmakefile:
>     Enable the new test cases.
> 
> tests/hard_coded/sub-module/g12_fe_bug.m:
>     Fix a type in a comment.

...

> diff --git a/doc/reference_manual.texi b/doc/reference_manual.texi
> index 328ed53..c47ff43 100644
> --- a/doc/reference_manual.texi
> +++ b/doc/reference_manual.texi
> @@ -4571,9 +4571,10 @@ interface section of its parent module.
>
>  If a sub-module is declared but not explicitly defined,
>  then there is an implicit definition with an empty implementation section
> -for that sub-module (this will result in an error, if the interface
> -section includes declarations but not definitions for any types,
> -predicates, modes, or (doubly) nested sub-modules).
> +for that sub-module.
> +This will result in an error if the interface section
> +includes declarations but not definitions for any types, insts, modes,
> +predicates, functions, or (doubly) nested sub-modules.

I know that's mostly the existing text but that's confusing because the
interface section cannot ever include definitions for functions or predicates
anyway.  To me that wording suggests that it could (i.e. a valid fix for the
error would be to add a clause for the function or predicate to the interface).
And if it's listing things that can be declared, why does it omit typeclass
and instance declarations?  (Presumably, because it was written before those
things were added to the language ...)

I suggest listing the cases separately, something like:

   For such sub-modules an error will occur if the interface contains any
   of the following:

   * a declaration for a function or predicate
   * a declaration for a type, inst, mode or typeclass that is not also
     defined in the interface section
   * a declaration for a type class instance
   * a (doubly) nested sub-module

The last bit concerning (doubly) nested sub-modules is a bit unclear too.
Surely I can have (doubly) nested interface-only sub-modules, provided
there is no violation of the previous three points.

The diff itself looks fine.

Cheers,
Julien.