[m-rev.] for review: fix singleton warnings for code with explicit quantification

Zoltan Somogyi zoltan.somogyi at runbox.com
Sat Jun 10 17:58:23 AEST 2023


On 2023-06-10 09:03 +02:00 CEST, "Julien Fischer" <jfischer at opturion.com> wrote:

>> > (An alternative design would be for make_hlds_warn.m to always nuke
>> any noempty list of quantified variables it traversed. However, this would
> 
> s/noempty/nonempty/
> 
>> require *always* rebuilding the clause body goal, which would probably
>> be slower on average.
> 
> You're missing a closing parenthesis there.

Both fixed.

>> The above is the main change in this diff. However, the change that is
>> responsible for the bulk of the diff is the addition of a flag to
>> exist_quant scopes to specify whether that scope was created by the user
>> or by the compiler. This is needed because if make_hlds_warn.m sees code
>> such as "some [Val] map.search(Map, Key, Val)", it definitely *should*
>> generate a warning about Val being singleton (if it does not occur outside
>> this code) if the "some [Val]" scope was put there by the compiler.
> 
> Do you mean was *not* put there by the compiler? That last sentence is
> not particularly clear.

No, I mean what I wrote. If the "some [Val]" was added by the compiler,
then what the user wrote was just "map.search(Map, Key, Val)", and under
the assumptions given above, Val *is* a singleton.

>> diff --git a/compiler/quantification.m b/compiler/quantification.m
>> index 7e41c73b9..a20c78994 100644
>> --- a/compiler/quantification.m
>> +++ b/compiler/quantification.m
>> @@ -87,8 +87,34 @@
>>  :- pred requantify_proc_general(nonlocals_to_recompute::in,
>>      proc_info::in, proc_info::out) is det.
>> > +:- type maybe_keep_quant
>> +    --->    do_not_keep_quant       % Throw away lists of quantified vars.
>> +    ;       keep_quant.             % Keep lists of quantified vars.
> 
> I would name that type "maybe_keep_quant_vars" and also append "_vars"
> to each of its constructors.

Done.


>> @@ -333,10 +365,13 @@ quantify_goal(NonLocalsToRecompute, Goal0, Goal, !Info) :-
>>          Goal = hlds_goal(!.GoalExpr, !.GoalInfo)
>>      ).
>> > -    % After this pass, explicit quantifiers are redundant, since all variables
>> -    % which were explicitly quantified have been renamed apart. So we don't
>> -    % keep them. We need to keep the structure, though, so that mode analysis
>> -    % doesn't try to reorder through quantifiers. (Actually it would make sense
> 
> Missing closing parenthesis.

It's not missing, its just that it is in unchanged text.

>> +    % If the qi_keep_quant is do_not_keep_quant, then we are past the compiler
> 
> s/qi_keep_quant/qi_keep_quant field/

Done.

> That looks fine otherwise.

Thank you.

Zoltan.


More information about the reviews mailing list