[m-dev.] diff: use_module

[Fergus Henderson]

Simon TAYLOR, you wrote:
> 
> Estimated hours taken: 13 
> 
> Implemented a :- use_module directive. This is the same as 
> :- import_module, except all uses of the imported items
> must be explicitly module qualified.

Ah, great stuff.  Thanks!

Question: does the code handle the following test case correctly,
when compiling with intermodule optimization?

	:- module foo.
	:- interface.
	:- pred p is det.
	:- implementation.
	:- import_module list.
	p :-
		list__merge_sort([1], _).

That should be an error, since `list__merge_sort' is not exported
from `list'.
But I'm a bit worried that your removal of `opt_decl' might mean
that the compiler will accept this code, rather than
reporting an error.

> compiler/prog_data.m
> 	Add a boolean argument to the `imported' pseudo-declaration which
> 	is yes if the following items must be module qualified.

I think it might be clearer if you added a `used' pseudo-declaration,
rather than changing the `imported' pseudo-declaration.

> -:- pred add_item_decl_pass_2(item, term__context, import_status, module_info,
> -			import_status, module_info,
> +:- pred add_item_decl_pass_2(item, term__context, pair(import_status, bool),
> +			module_info, pair(import_status, bool), module_info,
>  			io__state, io__state).

Can you please add a typedef for `pair(import_status, bool)'
and some documentation for it?

Or alternatively, it might be better to use a new type rather than `bool'.
The problem is that it is difficult to figure out what `yes' or `no'
is supposed to mean.

module_qual.m:

> -			unit	% junk slot
> +			bool	% is the current item from a module imported
> +				% using a use_module declaration or from an 
> +				% optimization interface.

The comment here doesn't say which is which.  What does `yes' mean?

> +	% If the current item is in the interface, remove the its module 
> +	% name from the list of modules not used in the interface.

s/the its/its/

> +		prog_out__write_context(Context),
> +		io__write_string("Error: "),
> +		{ set__to_sorted_list(BothSet, BothList) },
> +		( { BothList = [_] } ->
> +			io__write_string(" module "),
> +			prog_out__write_module_list(BothList),
> +			io__write_string(" is ")
> +		;
> +			io__write_string(" modules "),
> +			prog_out__write_module_list(BothList),
> +			io__write_string(" are ")
> +		),
> +		io__write_string("imported using both\n"),
> +		prog_out__write_context(Context),
> +		io__write_string("  `:- import_module' and `:- use_module' declarations.\n"),
> +

If this is an error, then you should call `io__set_exit_status(1)'.
But I think it should not be an error.
For example, it could make sense to use `use_module' in the
interface, and `import_module' in the implementation.
If both are specified, you should just treat it as equivalent
to `import_module'.

> +		% Treat the modules with both types of import as if they 
> +		% were imported using :- use_module.
> +		{ list__delete_elems(ImportedModules0, BothList,
> +			ImportedModules ) }

You should treat them as `:- import_module'.

typecheck.m:
> -make_pred_cons_info(TypeCheckInfo, PredId, PredTable, FuncArity,
> +make_pred_cons_info(_TypeCheckInfo, PredId, PredTable, FuncArity,
>  		_ModuleInfo, L0, L) :-
>  	map__lookup(PredTable, PredId, PredInfo),
>  	pred_info_arity(PredInfo, PredArity),
>  	pred_info_get_is_pred_or_func(PredInfo, IsPredOrFunc),
> -	pred_info_import_status(PredInfo, CalledStatus),
> -	typecheck_info_get_pred_import_status(TypeCheckInfo, CallingStatus),
>  	(
> -		% Only opt_imported preds can look at the declarations of
> -		% predicates from .opt files.
> -		( CalledStatus \= opt_decl
> -		; CallingStatus = opt_imported
> -		)

I think this is still necessary.  What if a module has both
foo/1 and foo/2, where foo/1 is exported but foo/2 is not,
but a declaration for foo/2 is put in the .opt file?

>  Declarations, predicate calls, types, modes and insts
>  can be explicitly module qualified using the `:' operator,
>  i.e. @code{module:name}. This is useful both for readability and
> -for resolving name conflicts.
> +for resolving name conflicts. Uses of entities imported using 
> + at code{use_module} declarations must be explicitly module qualified.

s/must/@emph{must}/

> +errors.m:000: Error:  module `int' is imported using both

s/  / /
(And fix the code that outputs that error message.)

-- 
Fergus Henderson <fjh at cs.mu.oz.au>   |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>   |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3         |     -- the last words of T. S. Garp.