[m-dev.] for review: pragma foreign_code for MC++ (part 1/2)

[Fergus Henderson]

On 10-Nov-2000, Tyson Dowd <trd at cs.mu.OZ.AU> wrote:
> +handle_return_value(CodeModel, PredOrFunc, Args0, ModuleInfo, Args, C_Code0) :-
...
> +	; CodeModel = model_semi,
> +		% we treat semidet functions the same as semidet predicates,
> +		% which means that for Mercury functions the Mercury return
> +		% value becomes the last argument, and the C return value
> +		% is a bool that is used to indicate success or failure.
> +		C_Code0 = "SUCCESS_INDICATOR = ",
> +		Args2 = Args0

After Zoltan's MR_ prefix change, that should perhaps become
MR_SUCCESS_INDICATOR.  Be careful with cvs merging here.

> Index: compiler/globals.m
...
> +convert_foreign_language("C", c).
> +convert_foreign_language("c", c).
> +convert_foreign_language("MC++", managedcplusplus).
> +convert_foreign_language("mc++", managedcplusplus).
...
> +foreign_language_string(managedcplusplus) = "MC++".

Hmm, "MC++" is a bit cryptic for those who don't know what it means.
It might be good to also allow a longer version which spells out the
name more fully, e.g. "Managed C++".

> +globals__io_lookup_foreign_language_option(Option, ForeignLang) -->
> +	globals__io_lookup_string_option(Option, String),
> +	{ convert_foreign_language(String, ForeignLang0) ->
> +		ForeignLang = ForeignLang0
> +	;
> +		error("globals__io_lookup_foreign_language_option: invalid foreign_language option")
> +	}.

There doesn't seem to be any code in your change which would prevent
that from calling error/1 if the user types in the wrong name.

You should add some code somewhere (probably in handle_options.m) to
check that the value specified for the --use-foreign-language option
is actually a valid language option, and if not you should report a
nice readable error message.

> +++ compiler/handle_options.m	2000/11/09 05:35:17
> @@ -568,6 +568,32 @@
>  	% we are expecting some to be missing.
>  	option_implies(use_opt_files, warn_missing_opt_files, bool(no)),
>  
> +
> +	% The preferred backend foreign language depends on the target.
> +	( 	
> +		{ Target = c },
> +		{ BackendForeignLanguage = "c" }
> +	;
> +		{ Target = il },
> +		{ BackendForeignLanguage = "mc++" }
> +	;
> +		% we don't generate java or handle it as a foreign
> +		% language just yet, but if we did...
> +		{ Target = java },
> +		{ BackendForeignLanguage = "java" }
> +	),

It would be better to use the canonical names here.  The canonical
names should be the ones that are properly capitalized.
Also if there are long and short alternatives (i.e.
"Managed C++" vs "MC++", you should use the long alternative here.

> +	globals__io_set_option(backend_foreign_language,
> +		string(BackendForeignLanguage)),
> +	% The default foreign language we use is the same as the backend.
> +	globals__io_lookup_string_option(use_foreign_language,
> +		UseForeignLanguage),
> +	( { UseForeignLanguage = "" } ->
> +		globals__io_set_option(use_foreign_language, 
> +			string(BackendForeignLanguage))
> +	;
> +		[]
> +	),

It would be no harder, I think, to make use_foreign_language an
accumulating list(string) option, thus allowing the user to give a
list of the languages desired in order of preference, rather than a
single string.

E.g. here instead of that if-then-else you can just have

	globals__io_lookup_string_option(use_foreign_language,
		UseForeignLanguage),
	globals__io_set_option(use_foreign_language, 
		UseForeignLanguage ++ [BackendForeignLanguage])

> +++ compiler/make_hlds.m	2000/11/08 06:39:31
> @@ -400,16 +400,17 @@
...
>  		% Handle pragma c_code decls later on (when we process
>  		% clauses).
> -		{ Pragma = foreign(_, _, _, _, _, _, _) },
> +		{ Pragma = foreign(_, _, _, _, _, _) },

The comment there has suffered documentation rot;
it should say something about foreign code, not c_code.

> +module_add_pragma_foreign_code(Attributes, PredName, PredOrFunc,
...
> +	globals__io_lookup_foreign_language_option(use_foreign_language,
> +		UseForeignLang),
...

To continue with my earlier remarks, that could be

	globals__get_foreign_languages_to_use(ForeignLangsToUse),

>  	;
> +			% Don't add clauses for foreign languages other
> +			% than the one we are using.
> +		{ foreign_language(Attributes, PragmaForeignLanguage) },
> +		{ UseForeignLang \= PragmaForeignLanguage }
> +	->

and that could be

		{ foreign_language(Attributes, PragmaForeignLanguage) },
		{ \+ list__member(PragmaForeignLanguage, ForeignLangsToUse) }
	->

> -warn_singletons_in_goal_2(pragma_foreign_code(_, _, _, _, _, ArgInfo, _,
> +warn_singletons_in_goal_2(pragma_foreign_code(_, _, _, _, ArgInfo, _,
>  		PragmaImpl), GoalInfo, _QuantVars, _VarSet, PredCallId, MI) --> 
>  	{ goal_info_get_context(GoalInfo, Context) },
>  	% XXX not just C code
> -	warn_singletons_in_pragma_c_code(PragmaImpl, ArgInfo, Context, 
> +	warn_singletons_in_pragma_foreign_code(PragmaImpl, ArgInfo, Context, 
>  		PredCallId, MI).

Is the XXX still needed?

> +	% warn_singletons_in_pragma_foreign_code checks to see if each
> +	% variable is mentioned at least once in the foreign code
> +	% fragments that ought to mention it. If not, it gives a
> +	% warning.
> +	% (note that for some foreign languages it might not be
> +	% appropriate to do this check, or you may been to add a
> +	% transformation to map Mercury variable names into identifiers
> +	% for that foreign language).

s/been/need/ ?

s/(note/(Note/

> +:- pred warn_singletons_in_pragma_foreign_code(pragma_foreign_code_impl,
>  	list(maybe(pair(string, mode))), prog_context, simple_call_id,
>  	module_info, io__state, io__state).
> -:- mode warn_singletons_in_pragma_c_code(in, in, in, in, in, di, uo) is det.
> +:- mode warn_singletons_in_pragma_foreign_code(in, in, in, in, in,
> +	di, uo) is det.
>  
> -warn_singletons_in_pragma_c_code(PragmaImpl, ArgInfo, 
> +warn_singletons_in_pragma_foreign_code(PragmaImpl, ArgInfo, 
>  		Context, PredOrFuncCallId, ModuleInfo) -->
>  	(
>  		{ PragmaImpl = ordinary(C_Code, _) },
> @@ -4848,18 +4688,18 @@
>  			io__stderr_stream(StdErr1),
>  			io__set_output_stream(StdErr1, OldStream1),
>  			prog_out__write_context(Context),
> -			io__write_string("In `:- pragma c_code' for "),
> +			io__write_string("In `:- pragma foreign_code' for "),
>  			hlds_out__write_simple_call_id(PredOrFuncCallId),
>  			io__write_string(":\n"),
>  			prog_out__write_context(Context),
>  			( { UnmentionedVars = [_] } ->
>  				io__write_string("  warning: variable `"),
>  				write_string_list(UnmentionedVars),
> -				io__write_string("' does not occur in the C code.\n")
> +				io__write_string("' does not occur in the foreign code.\n")

I think it would be nicer to say "in the %s code", where %s is
the name of the language, rather than "in the foreign code".

>  			;
>  				io__write_string("  warning: variables `"),
>  				write_string_list(UnmentionedVars),
> -				io__write_string("' do not occur in the C code.\n")
> +				io__write_string("' do not occur in the foreign code.\n")

Likewise here.

> +:- pred clauses_info_add_pragma_foreign_code(
> +	clauses_info, purity, pragma_foreign_code_attributes, pred_id,
> +	proc_id, prog_varset, list(pragma_var), list(type),
> +	pragma_foreign_code_impl, prog_context, pred_or_func, sym_name,
> +	arity, clauses_info, module_info, module_info, qual_info,
> +	qual_info, io__state, io__state) is det.
> +:- mode clauses_info_add_pragma_foreign_code(in, in, in, in, in, in, in,
> +	in, in, in, in, in, in, out, in, out, in, out, di, uo) is det.

I think it would be better to use the combined pred-mode declaration
syntax here.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
                                    |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------