[m-dev.] for review: recognise associativity assertions

[Fergus Henderson]

On 17-Nov-1999, Peter Ross <petdr at cs.mu.OZ.AU> wrote:
> Recognise associativity assertions, and use them to introduce
> accumulators.
...
> mercury/compiler/accumulator.m:
>     Rather than failing if a call is associative and nothing is known
>     about the effect of rearranging the argument order, report a
>     suppressible warning.

Hmm, interesting...

What can the user do if for one warning, they want to not just
suppress the warning but also prevent the argument rearrangement,
while for another warning, they want to just supress the warning?

> +++ accumulator.m	1999/11/17 03:53:30
> @@ -222,8 +230,46 @@
>  			[]
>  		),
>  
> -		{ ProcInfo   = ProcInfo1 },
> -		{ ModuleInfo = ModuleInfo1 }
> +		globals__io_lookup_bool_option(inhibit_accumulator_warnings,
> +				InhibitWarnings),
> +		(
> +			( { Warnings = [] } ; { InhibitWarnings = yes } )
> +		->
> +			{ ModuleInfo = ModuleInfo1 }
> +		;
> +			{ error_util__describe_one_pred_name(ModuleInfo1,
> +					PredId, PredName) },
> +			{ pred_info_context(PredInfo, Context) },
> +
> +			error_util__write_error_pieces(Context, 0,
> +					[words("In the"), words(PredName)]),

Probably you should delete the word "the" their, for consistency
with error messages from other parts of the compiler.

> +			{ proc_info_varset(ProcInfo, VarSet) },
> +			accumulator__warnings(Warnings, VarSet, ModuleInfo1),
> +
> +			error_util__write_error_pieces(Context, 2, 
> +					[
> +					words("Please ensure that these"),
> +					words("argument rearrangements"),
> +					words("do not introduce"),
> +					words("performance problems."),
> +					words("These warnings can"),
> +					words("be supressed by"),
> +					words("--inhibit-accumulator-warnings.")

Put the option name inside `single quotes'.

> +:- pred accumulator__warning(warning::in, prog_varset::in, module_info::in,
> +		prog_context::out, list(format_component)::out) is det.
> +
> +accumulator__warning(w(Context, PredId, VarA, VarB), VarSet, ModuleInfo,
> +		Context, Formats) :-
> +	error_util__describe_one_pred_name(ModuleInfo, PredId, PredStr),
> +	varset__lookup_name(VarSet, VarA, VarAStr),
> +	varset__lookup_name(VarSet, VarB, VarBStr),
> +	Formats = [words("warning: The"), words(PredStr),
> +			words("has had the location of the variables"),
> +			words(VarAStr), words("and"), words(VarBStr),
> +			words("swapped to allow accumulator introduction.")
> +			].

I think you should probably replace "The" with "the call to".

I can anticipate that this warning will confuse a lot of users,
so you should definitely include a verbose expanation that is
enabled by --verbose-errors.

It would be a good idea to have a test case in tests/warnings
to test this warning.

> +assertion__is_associativity_assertion(AssertId, Module, CallVars,
> +		AssociativeVars) :-
> +	assertion__goal(AssertId, Module, Goal - GoalInfo),
> +	equivalent(Goal - GoalInfo, P, Q),
> +
> +	goal_info_get_nonlocals(GoalInfo, UniversiallyQuantifiedVars),
> +
> +	P = some(_, _, conj(PCalls) - _) - _PGoalInfo,
> +	Q = some(_, _, conj(QCalls) - _) - _QGoalInfo,
> +
> +	AssociativeVars = promise_only_solution(associative(PCalls, QCalls,
> +				UniversiallyQuantifiedVars, CallVars)).

I think the code here should have a comment explaining why the
call to promise_only_solution is needed and why it is guaranteed
to only have one solution (if, indeed, it is -- from a brief perusal
of the code it wasn't immediately clear to me that that was the case).

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