[m-rev.] for review: fix handling of transitive module imports

[Fergus Henderson]

That looks great, Simon.

On 02-May-2001, Simon Taylor <stayl at cs.mu.OZ.AU> wrote:
> 
> Fix a bug in the module import mechanism -- use_module should
> not be transitive. This change is needed for smart recompilation
> to avoid needing to check whether the removal of a transitive
> import could cause compilation errors -- it never should.

You should add something like

    Also some bugs in the handling of type classes.

to the summary here, since many of the changes below
are fixing bugs with typeclasses rather than with the
above-mentioned bug with transitive imports:

	> compiler/prog_data.m:
	> 	Fix the representation of type class bodies. 
	> 	`:- typeclass foo where [].' declares a typeclass with no
	> 	methods. `:- typeclass foo.' declares an abstract typeclass.
	> 	The old representation made no distinction between these cases.
	> 
	> compiler/modules.m:
	> 	Remove the bodies of typeclass declarations placed in `.int2'
	> 	files -- the methods should not be available unless the module
	> 	is explicitly imported.
	> 
	> compiler/equiv_type.m:
	> compiler/mercury_to_mercury.m:
	> compiler/prog_io_typeclass.m:
	> 	Handle the change to the representation of typeclass bodies.
	> 
	> compiler/prog_io_typeclass.m:
	> 	Check that the arguments of a typeclass declaration
	> 	are distinct variables.
	> 
	> compiler/make_hlds.m:
	> 	Handle abstract typeclass declarations.
	> 
	> compiler/check_typeclass.m:
	> 	Check that all typeclasses have a definition somewhere.
	> 
	> compiler/intermod.m:
	> 	Write abstract_exported typeclasses to the `.opt' file.


> Index: NEWS
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/NEWS,v
> retrieving revision 1.206
> diff -u -u -r1.206 NEWS
> --- NEWS	2001/03/27 05:23:01	1.206
> +++ NEWS	2001/05/02 05:44:40
> @@ -15,6 +15,15 @@
>  * The exception module has a new predicate `try_store', which is
>    like `try_io', but which works with stores rather than io__states.
>  
> +Changes to the Mercury implementation:
> +* We've fixed a bug in the handling of module imports. Previously,
> +  if `module1' imported `module2' which imported `module3' in
> +  its interface section, types, insts, modes and typeclasses defined
> +  in the interface of `module3' could be used in `module1' even
> +  if `module1' did not import `module3' directly.
> +
> +  This change will break some existing programs, but that is easily fixed
> +  by adding any necessary `:- import_module' or `:- use_module' declarations.

I suggest a couple of minor changes:
s/fix a bug/fixed a long-standing bug/
s/types, insts/then any types, insts/

> Index: compiler/module_qual.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/module_qual.m,v
> retrieving revision 1.65
> diff -u -u -r1.65 module_qual.m
> --- compiler/module_qual.m	2001/04/03 03:20:06	1.65
> +++ compiler/module_qual.m	2001/05/01 07:17:37
> @@ -182,7 +182,8 @@
>  	% so we use a simpler data type here than hlds_pred__import_status.
>  :- type import_status
>  	--->	exported
> -	;	not_exported.
> +	;	not_exported
> +	;	imported.

Hmm, if `not_exported' doesn't include the `imported' cases,
wouldn't it be better to rename `no_exported' as `local'?

> Index: compiler/prog_data.m
> @@ -866,6 +870,13 @@
>  		% This is used internally by the compiler,
>  		% to identify items which originally
>  		% came from a .opt file.
> +	;	transitively_imported
> +		% This is used internally by the compiler,
> +		% to identify items which originally
> +		% came from a .opt file.
> +		% came from a `.opt' or `.int2' file.
> +		% These should not be allowed to
> +		% match items in the current module.

You should document here that unlike `:- interface', `:- implementation',
or the other pseudo-declarations `:- imported(interface)', etc.,
this does not define a new section of the module which lasts until
the next pseudo-declaration.  Instead, everything which follows
a `transitively_imported' marker is treated as transitively imported,
regardless of what other pseudo-declarations follow.

Incidentally, why is that?  It would be more consistent with the
way that the rest of the module import stuff works for
`transitively_imported' to define a new section.
It would be nicer if we could keep things consistent.
(Don't worry about this if it would be a lot of work.
But if it's an easy change, I think it's worth doing.)

Apart from that, this change looks great.  Thanks.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
                                    |  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
--------------------------------------------------------------------------