[m-rev.] diff: mercury versions of library pragma foreign_procs

[Fergus Henderson]

In general a diff this size should be reviewed before being committed, IMHO.

The basic idea of the change is good, but there are a large number of
issues in it that need to be addressed -- see below.
Could you please either address these issues promptly,
or back out your change until such time as they can be addressed?


There should be comments on all those calls to sorry/1, explaining that
this is just a default for back-ends that don't support the C interface.
Otherwise the code is potentially misleading to the reader -- it looks
like that predicate isn't implemented at all.

On 11-Jun-2002, Peter Ross <pro at missioncriticalit.com> wrote:
> @@ -3212,22 +3207,28 @@
>  	ascii_encoder =	new System::Text::ASCIIEncoding();
>  ").
>  
> +io__gc_init(_, _) -->
> +	{ private_builtin__sorry("io__gc_init") }.

This one should just default to doing nothing.

>  :- pred io__stream_init(io__state, io__state).
>  :- mode io__stream_init(di, uo) is det.
>  
> -:- pragma foreign_proc("MC++", 
> +:- pragma foreign_proc("C", 
>  		io__stream_init(IO0::di, IO::uo), [will_not_call_mercury,
>  			promise_pure], "
> -	ascii_encoder =	new System::Text::ASCIIEncoding();
>  	update_io(IO0, IO);
>  ").
>  
> -:- pragma foreign_proc("C", 
> +:- pragma foreign_proc("MC++", 
>  		io__stream_init(IO0::di, IO::uo), [will_not_call_mercury,
>  			promise_pure], "
> +	ascii_encoder =	new System::Text::ASCIIEncoding();
>  	update_io(IO0, IO);
>  ").
>  
> +io__stream_init -->
> +	{ private_builtin__sorry("io__stream_init") }.

This procedure appears to be unreferenced, and should probably just
be deleted (although this can be done as a separate change if you prefer).

> @@ -5026,6 +5144,7 @@
>  	update_io(IO0, IO);
>  ").
>  
> +/*
>  :- pragma foreign_proc("MC++",
>  	io__call_system_code(Command::in, Status::out, _Msg::out,
>  			IO0::di, IO::uo),
> @@ -5041,25 +5160,27 @@
>  	Status = NULL;
>  	update_io(IO0, IO);
>  ").
> +*/

There should be a comment before the :- pragma explaining why this
code is commented out.

> @@ -5282,6 +5403,7 @@
>  	update_io(IO0, IO);
>  }").
>  
> +/*
>  :- pragma foreign_proc("MC++",
>  	io__remove_file_2(FileName::in, RetVal::out, RetStr::out,
>  		IO0::di, IO::uo),

Likewise (although in this case, this is a problem with the
existing code).

> +++ library/math.m	11 Jun 2002 10:16:40 -0000
> @@ -264,6 +264,10 @@
>  #endif
>  ").
>  
> +domain_checks :-
> +	semidet_succeed,
> +	private__builtin__sorry("domain_checks").

s/private__builtin/private_builtin/

Likewise throughout this file.

> +++ library/rtti_implementation.m	11 Jun 2002 10:16:42 -0000
> @@ -127,19 +127,24 @@
>  :- type pseudo_type_info ---> pred_type(c_pointer).
>  
>  :- pragma foreign_proc("C#",
> -	get_type_info(_T::unused) = (TypeInfo::out),
> +	get_type_info(T::unused) = (TypeInfo::out),
>  	[will_not_call_mercury, promise_pure, thread_safe],
>  " 
> +	// T
>  	TypeInfo = TypeInfo_for_T;
>  ").
>  
>  :- pragma foreign_proc("C",
> -	get_type_info(_T::unused) = (TypeInfo::out), 
> +	get_type_info(T::unused) = (TypeInfo::out), 
>  	[will_not_call_mercury, promise_pure, thread_safe],
>  "
> +	/* T */
>  	TypeInfo = TypeInfo_for_T;
>  ").

This change was not mentioned in the log message.
It looks like a bad idea to me.

> @@ -1434,6 +1460,15 @@
>  	cc_multi_equal(X::di, Y::uo),
>  	[will_not_call_mercury, thread_safe, promise_pure],
>  	"Y = X;").
> +
> +semidet_succeed :-
> +	private_builtin__sorry("semidet_succeed").
> +semidet_fail :-
> +	private_builtin__sorry("semidet_fail").

These should just call `true' and `fail' respectively.

> +:- pragma promise_pure(cc_multi_equal/2).
> +cc_multi_equal(_, _) :-
> +	private_builtin__sorry("cc_multi_equal").

That should just call =/2.

> +++ library/store.m	11 Jun 2002 10:16:44 -0000
> @@ -250,8 +250,12 @@
>  :- pred store__do_init(store(some_store_type)).
>  :- mode store__do_init(uo) is det.
>  
> -:- pragma foreign_proc("C", store__do_init(_S0::uo),
> -	[will_not_call_mercury, promise_pure], "").
> +:- pragma foreign_proc("C", store__do_init(S0::uo),
> +	[will_not_call_mercury, promise_pure],
> +	"/* XXX mention S0 to avoid warning */").

That change is not mentioned in the log message
and IMHO is a bad idea.

> @@ -443,12 +465,28 @@
>  ").
>  
>  :- pragma foreign_proc("C",
> -	extract_ref_value(_S::di, Ref::in, Val::out),
> +	extract_ref_value(S::di, Ref::in, Val::out),
>  		[will_not_call_mercury, promise_pure],
>  "
> +	/* XXX mention S to avoid warning. */
>  	Val = * (MR_Word *) Ref;
>  ").

Likewise.

> @@ -1116,6 +1129,9 @@
>  	Index = WholeString->IndexOf(SubString);
>  }").
>  
> +string__sub_string_search(_, _, _) :-
> +	private_builtin__sorry("string__sub_string_search").

An XXX comment is warranted here, IMHO,
since string__sub_string_search should be default
be implemented in Mercury using string__length and
string__index.

> @@ -1817,6 +1858,8 @@
>  		[will_not_call_mercury, promise_pure, thread_safe], "
>  	SUCCESS_INDICATOR = (Str->IndexOf(Ch) != -1);
>  ").
> +string__contains_char(_, _) :-
> +	private_builtin__sorry("string__contains_char").

Likewise here.

> Index: library/type_desc.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/library/type_desc.m,v
> retrieving revision 1.9
> diff -u -r1.9 type_desc.m
> --- library/type_desc.m	21 May 2002 08:09:58 -0000	1.9
> +++ library/type_desc.m	11 Jun 2002 10:16:48 -0000
> @@ -477,9 +477,10 @@
>  	% Prototypes and type definitions.
>  
>  :- pragma foreign_proc("C",
> -	type_of(_Value::unused) = (TypeInfo::out),
> +	type_of(Value::unused) = (TypeInfo::out),
>  	[will_not_call_mercury, thread_safe, promise_pure],
>  "{
> +	/* Value */
>  	TypeInfo = TypeInfo_for_T;
>  
>  	/*

That change is not mentioned in the log message
and IMHO is a bad idea.

> @@ -498,26 +499,35 @@
>  }").
>  
>  :- pragma foreign_proc("C#",
> -	type_of(_Value::unused) = (TypeInfo::out),
> +	type_of(Value::unused) = (TypeInfo::out),
>  	[will_not_call_mercury, thread_safe, promise_pure],
>  "
> +	// Value
>  	TypeInfo = TypeInfo_for_T;
>  ").

Likewise.

>  :- pragma foreign_proc("C", 
> -	has_type(_Arg::unused, TypeInfo::in),
> +	has_type(Arg::unused, TypeInfo::in),
>  	[will_not_call_mercury, thread_safe, promise_pure],
>  "
> +	/* Arg */
>  	TypeInfo_for_T = TypeInfo;
>  ").

Likewise.

>  :- pragma foreign_proc("C#", 
> -	has_type(_Arg::unused, TypeInfo::in),
> +	has_type(Arg::unused, TypeInfo::in),
>  	[will_not_call_mercury, thread_safe, promise_pure],
>  "
> +	// Arg
>  	TypeInfo_for_T = TypeInfo;
>  ").

Likewise.

-- 
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
--------------------------------------------------------------------------