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

[Fergus Henderson]

On 03-May-2002, Peter Ross <peter.ross at miscrit.be> wrote:
> +++ tests/hard_coded/foreign_type.m	3 May 2002 17:45:28 -0000
> +:- pragma foreign_proc(c, new(X::in, Y::in) = (C::out),
> +	[will_not_call_mercury, promise_pure],
> +"
> +	C = GC_NEW(coord);
> +	C->x = X;
> +	C->y = Y;
> +").

GC_NEW should be MR_GC_NEW.

> +++ foreign/doc/reference_manual.texi	3 May 2002 17:45:27 -0000
> @@ -5484,9 +5484,36 @@
> +The @var{CForeignType} can be any C type name that obeys the following
> +restrictions.
> +The following snippet of C code must evaluate to true
> + at code{sizeof(CForeignType) == sizeof(void *)},
> +if not the result of using the foreign type is undefined.

The comma here is not grammatically correct.
I suggest you change that to

	The C expression @samp{sizeof(CForeignType) == sizeof(void *)} must
	evaluate to true; if not, the result of using the foreign type is
	undefined.

> +The type name must be such that no part of it is required after a
> +variable name to be valid C.

This is not really clear.

> +Currently only integer and pointer types are accepted as foreign_types,
> +at a later date we plan to lift this restriction and allow enum, struct
> +and float types.

The first comma here is not grammatically correct; it should be a semi-colon.

s/float/floating/
(We plan to allow `double' and `long double' too, right?)

> @@ -2030,6 +2037,13 @@
>  		io__write_string(VarName),
>  		io__write_string(")")
>  	;
> +		% Note that for this cast to be correct the foreign type
> +		% must be word sized.
> +		( { MaybeForeignType = yes(_) } ->
> +			output_llds_type_cast(word)
> +		;
> +			[]
> +		),

The comment here is misleading because being word sized is not enough --
it needs to be an integer type or a pointer type.

> Index: foreign/compiler/make_hlds.m
> @@ -1921,9 +1923,23 @@
>  				module_info_set_types(Module0, Types, Module)
>  			}
>  		;
> -			{ Module = Module0 },
> -			multiple_def_error(Status, Name, Arity, "type",
> -				Context, OrigContext, _)
> +			{ merge_foreign_type_bodies(Body, Body_2, NewBody) }
> +		->
> +			{ hlds_data__set_type_defn(TVarSet_2, Params_2,
> +				NewBody, Status, Context, T3) },
> +			{ map__det_update(Types0, TypeCtor, T3, Types) },
> +			{ module_info_set_types(Module0, Types, Module) }
> +		;
> +			% otherwise issue an error message if the second
> +			% definition wasn't read while reading .opt files. 
> +			{ Status = opt_imported }
> +		->
> +			{ Module = Module0 }
> +		;
> +				% XXX Fix this merge up.
> +			{ module_info_incr_errors(Module0, Module) },
> +			multiple_def_error(Status, Name, Arity, "type", Context,
> +				OrigContext, _)

I don't understand the XXX comment here.

Otherwise that looks OK.

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