[m-rev.] for review: C foreign types

[Peter Ross]

On Mon, Mar 11, 2002 at 10:59:45PM +1100, Fergus Henderson wrote:
> On 05-Mar-2002, Peter Ross <peter.ross at miscrit.be> wrote:
> > 
> > +++ doc/reference_manual.texi	5 Mar 2002 12:05:48 -0000
> > +The C @samp{pragma foreign_type} declaration is of the form:
> > +
> > + at example
> > +:- pragma foreign_type(c, @var{MercuryTypeName}, @var{CForeignType}).
> > + at end example
> > +
> > +The @var{CForeignType} can be any C type name that obeys the following
> > +restrictions.
> > +The type must fit into a machine word.
> 
> The notion "machine word" is not defined.
> 
I have changed it to sizeof(CForeignType) == sizeof(char *).

> Function types and incomplete types should not be allowed.
> 
> Array types should probably not be allowed (due to their
> non-orthogonal semantics for parameter-passing etc. in C).
> 
> Enum types, struct types, and floating point types should ideally be allowed,
> but your implementation only works for integer types and pointer types.
> 
It would be ideal if it also worked on the above types.
Currently I do not have the time to implement the boxing and unboxing of
values automatically.
Personally I think it would be a great thing to have, as it would allow
us (once we could specify a user-defined comparison) to move the builtin
types out of the compiler and into the library.
So for the moment, I believe, that forcing the user to do there own
boxing and unboxing is a sufficient amount of functionality for most
peoples uses.
Does that sound find to you?

> > Index: compiler/foreign.m
> ...
> >  to_exported_type(ModuleInfo, Type) = ExportType :-
> ...
> > +		( Body = foreign_type(MaybeIL, MaybeC) ->
> > +			( Target = c,
> > +				( MaybeC = yes(c(NameStr)),
> > +					Name = unqualified(NameStr)
> > +				; MaybeC = no,
> > +					error("to_exported_type: no C type")
> > +				)
> > +			; Target = il, 
> > +				( MaybeIL = yes(il(_, _, Name))
> > +				; MaybeIL = no,
> > +					error("to_exported_type: no IL type")
> > +				)
> > +			; Target = java,
> > +				error("to_exported_type: java NYI")
> > +			; Target = asm,
> > +				( MaybeC = yes(c(NameStr)),
> > +					Name = unqualified(NameStr)
> > +				; MaybeC = no,
> > +					error("to_exported_type: no C type")
> > +				)
> > +			),
> 
> These should all call `sorry', rather than `error'.
> And the error messages should be better; users might well encounter them.
> 
> Or alternative, if this is supposed to be checked properly
> at some earlier stage in the front-end, use `unexpected'
> rather than `sorry' or `error', and put a comment explaining why.
> 
All the calls to error will be changed to unexpected, as this test is
checked earlier.

> > Index: compiler/make_hlds.m
> > @@ -794,6 +791,61 @@
> >  		add_pragma_type_spec(Pragma, Context, Module0, Module,
> >  			Info0, Info)
> >  	;
> > +		{ Pragma = foreign_type(_, _, Name) }
> > +	->
> > +		{ TypeId = Name - 0 },
> > +		{ module_info_types(Module0, Types) },
> > +		{ TypeStr = error_util__describe_sym_name_and_arity(
> > +				Name / 0) },
> > +		( 
> > +		    { map__search(Types, TypeId, Defn) },
> > +		    { hlds_data__get_type_defn_body(Defn, Body) },
> > +		    { Body = foreign_type(MaybeIL, MaybeC) }
> > +		->
> > +		    { module_info_globals(Module0, Globals) },
> > +		    { globals__lookup_bool_option(Globals, target_code_only,
> > +			    TargetCode) },
> > +		    ( { TargetCode = yes } ->
> > +			{ globals__get_target(Globals, Target) },
> > +			( { Target = c },
> > +			    ( { MaybeC = yes(_) },
> > +			    	{ Module = Module0 }
> > +			    ; { MaybeC = no },
> > +				{ ErrorPieces = [
> > +				    words("Error: No C pragma"),
> > +				    words("foreign_type for"),
> > +				    fixed(TypeStr)
> > +				] },
> > +				error_util__write_error_pieces(Context,
> > +					0, ErrorPieces),
> > +				{ module_info_incr_errors(Module0, Module) }
> > +			    )
> > +			; { Target = il },
> > +			    ( { MaybeIL = yes(_) },
> > +			    	{ Module = Module0 }
> > +			    ; { MaybeIL = no },
> > +				{ ErrorPieces = [
> > +				    words("Error: No IL pragma"),
> > +				    words("foreign_type for"),
> > +				    fixed(TypeStr)
> > +				] },
> > +				error_util__write_error_pieces(Context,
> > +					0, ErrorPieces),
> > +				{ module_info_incr_errors(Module0, Module) }
> > +			    )
> > +			; { Target = java },
> > +			    	{ Module = Module0 }
> > +			; { Target = asm },
> > +			    	{ Module = Module0 }
> > +			)
> > +		    ;
> > +			{ Module = Module0 }
> > +		    )
> > +		;
> > +		    { error("add_item_clause: unable to find foreign type") }
> > +		),
> > +		{ Info = Info0 }	
> 
> This code should all be in a sub-routine.
> 
> Why are you checking the value of the `target_code_only' option here?
> That should be completely irrelevant.  Checking that here is wrong.
> Maybe there is something you need to check, but `target_code_only' is not it.
> 
This is the code that checks that there is pragma foreign_type for the
backend target that you are compiling to.  I was under the impression
that target_code_only was set whenever you were actually compiling to a
specific backend.  What I was worried about was having the check be
enabled when for example you were writing out the interface file.

Do you know what option I should check?
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------