[m-dev.] for review: type specialization (relative) [1]

[Fergus Henderson]

On 20-Apr-1999, Simon Taylor <stayl at cs.mu.OZ.AU> wrote:
> 
> --- ./compiler/higher_order.m	Tue Apr 20 12:02:41 1999
> +++ /home/pgrad/stayl/mercury0/compiler/higher_order.m	Thu Apr 15 12:17:11 1999
> +				error(
> +	"higher_order.m: can't find `instance-constraint_from_typeclass_info'")

s/-/_/

> +	% Build calls to
> +	% `private_builtin:instance_constraint_from_typeclass_info/3'
> +	% to extract the typeclass_infos for the constraints on an instance.
> +	% This simulates the action of `do_call_*_class_method' in
> +	% runtime/mercury_ho_call.c.

Zoltan has replace do_call_{det,semi,nondet}_class_method
with do_call_class_method, so

	s/_*_/_/

> +			(
> +				% This handles the introduced predicates
> +				% which call class methods. Without this,
> +				% user-specified specialized versions of
> +				% class methods won't be called.
> +				UserTypeSpec = yes,
> +				(
> +					pred_info_get_markers(CalledPredInfo,
> +						Markers),
> +					check_marker(Markers, class_method)
> +				;
> +					pred_info_name(CalledPredInfo,
> +						CalledPredName),
> +					string__prefix(CalledPredName,
> +						"Introduced_")
> +				)

The hard-coded string "Introduced_" should be
"Introduced_pred_for_", for consistency with the
place where those prefixes are created.

I think it would be helpful to say "the predicates introduced
by check_typeclass.m" rather than "the introduced predicates".

> +filter_requests_2(_, _, [], _, Requests, Requests) --> [].
> +filter_requests_2(Params, ModuleInfo, [Request | Requests0],
...
> +	maybe_write_request(VeryVerbose, ModuleInfo, "% Request for",
> +		qualified(PredModule, PredName), Arity, ActualArity,
> +		no, HOArgs),
...
> +		->
> +			{ FilteredRequests1 = [Request | FilteredRequests0] }
> +		;
> +			{ FilteredRequests1 = FilteredRequests0 },
> +			maybe_write_string(VeryVerbose,
> +			"% Not specializing (recursive specialization):\n")
> +		)
> +	;
> +		{ FilteredRequests1 = FilteredRequests0 },
> +		maybe_write_string(VeryVerbose,
> +			"% Not specializing (goal too large):\n")
> +	),
> +	filter_requests_2(Params, ModuleInfo, Requests0, GoalSizes,
> +		FilteredRequests1, FilteredRequests).

It looks to me like the colons (":") in those messages are in the
wrong place.  What's supposed to come after the colon?
Doesn't the request get printed _before_ the "Not specializing"
message?

> --- ./compiler/hlds_goal.m	Mon Mar 22 19:07:11 1999
> +++ /home/pgrad/stayl/mercury0/compiler/hlds_goal.m	Thu Apr  8 12:40:40 1999
> +	%
> +	% Produce a goal to construct a given constant.
> +	%
> +
> +:- pred make_int_const_construction(prog_var, int, hlds_goal).
> +:- mode make_int_const_construction(in, in, out) is det.
> +
> +:- pred make_string_const_construction(prog_var, string, hlds_goal).
> +:- mode make_string_const_construction(in, in, out) is det.

You should document which parts of the hlds_goal these predicates
will produce correct information for.  Do they fill in the
modes, instmap delta, etc.?

> diff --recursive -u ./compiler/make_hlds.m /home/pgrad/stayl/mercury0/compiler/make_hlds.m
> --- ./compiler/make_hlds.m	Tue Apr 20 12:01:19 1999
> +++ /home/pgrad/stayl/mercury0/compiler/make_hlds.m	Tue Mar 30 17:00:44 1999
...
> +	% Check that the type substitution for a `:- pragma type_spec'
> +	% declaration is valid.
> +	% A type substitution is invalid if:
> +	%	- it substitutes unknown type variables
> +	% 	- it substitutes existentially quantified type variables
> +	% 	- the replacement types are not ground

The documentation here is misleading.
The last condition is only a warning, not an error.

>  report_non_ground_subst(PredInfo0, Context) -->
>  	report_pragma_type_spec(PredInfo0, Context),
> -	io__write_string("  error: the substitution does not make the\n"),
> -	io__nl,
>  	prog_out__write_context(Context),
> -	io__write_string("  substituted types ground.\n").
> +	io__write_string(
> +		"  warning: the substitution does not make the substituted\n"),
> +	prog_out__write_context(Context),
> +	io__write_string("  types ground. The declaration will be ignored.\n").

There should be something in the warning message to indicate that this
is a limitation of the current implementation.

Apart from the issues mentioned above, the relative diff looks fine.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- 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
--------------------------------------------------------------------------