[m-rev.] for review: foreign type imports

Tyson Dowd trd at cs.mu.OZ.AU
Thu Oct 25 15:13:20 AEST 2001


On 24-Oct-2001, Peter Ross <peter.ross at miscrit.be> wrote:
> Hi,
> 
> For Tyson to review.
> 
> With this change the main branch is now able to use the intgen tool
> provided you re-enable parsing of the prog_io pragma.
> 
> ===================================================================
> 
> 
> Estimated hours taken: 0.5
> Branches: main
> 
> Port the change to handle foreign imports onto the main branch.
> 
> The foreign_type pragma allows one to specify where a declaration for a
> foreign type is located, thus this import must be output in the correct
> source module.

You need to improve this log message to be much more precise. 

What declaration are you talking about?  Isn't foreign_type declaring a
type?  
What import?  When did foreign types start implying imports?  Why?
What source module are you talking about?  The Mercury source module is
what seems to be the most obvious meaning, but I doubt it is what you
meant.

Perhaps an example would make it clearer...

> compiler/mlds.m:
>     Change the import type so that as well as the name of the import it
>     also includes whether or not this import is of a file generated by
>     the mercury system.

Fine.

>     
> compiler/ml_code_gen.m:
>     For each foreign type add its location to the list of mlds imports.

I'm not very keen on the way this is done.
I feel the code in this section is very simple because it is just a hack
-- what if a type requires 2 imports or more for some case in the future?
Or if the imports can't be just turned into mlds_module_names so easily?

A more general mechanism would add foreign imports as part of the HLDS,
and then process those imports and add them to the MLDS here. 

Also we should allow different sorts of foreign imports in the MLDS,
instead of just putting a flag on ordinary imports.

But it's a bit of work for no immediate gain, so I'm not sure whether it
is worth doing right now.

> compiler/mlds_to_il.m:
>     Changes due to the new mlds__import definition.
>     Only output calls to the class constructors for those modules which
>     are mercury modules.
> 
> compiler/mlds_to_c.m:
> compiler/mlds_to_java.m:
>     Changes due to the new mlds__import definition.
> 
> 

> Index: compiler/mlds.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/mlds.m,v
> retrieving revision 1.72
> diff -u -r1.72 mlds.m
> --- compiler/mlds.m	24 Oct 2001 13:34:28 -0000	1.72
> +++ compiler/mlds.m	24 Oct 2001 15:05:04 -0000
> @@ -321,9 +321,11 @@
>  % Currently an import just gives the name of the package to be imported.
>  % This might perhaps need to be expanded to cater to different kinds of
>  % imports, e.g. imports with wild-cards ("import java.lang.*").
> -:- type mlds__import == mlds_module_name.
> -					% Specifies the name of a package or
> -					% class to import.
> +:- type mlds__import
> +	--->	import(
> +			import_name		:: mlds_module_name,
> +			is_mercury_import	:: bool
> +		).
>  
>  % An mlds_module_name specifies the name of an mlds package or class.
>  :- type mlds_module_name.

I think a discriminated union might be a better choice here

	:- type mlds__import
		---> mercury_import(import_name :: mlds_module_name)
		;    foreign_import(foreign_import_name ::
			foreign_import_name).

	:- type foreign_import_name
		---> il_assembly_name(mlds_module_name).

And we can add other kinds of imports to foreign_import_name... 


The rest of the change is ok and fairly simple.

-- 
       Tyson Dowd           # 
                            #  Surreal humour isn't everyone's cup of fur.
     trd at cs.mu.oz.au        # 
http://www.cs.mu.oz.au/~trd #
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------



More information about the reviews mailing list