[PATCH] opts: improve option suggestion
Martin Liška
mliska@suse.cz
Thu May 12 07:10:37 GMT 2022
On 5/11/22 20:49, David Malcolm wrote:
> On Wed, 2022-05-11 at 16:49 +0200, Martin Liška wrote:
>> In case where we have 2 equally good candidates like
>> -ftrivial-auto-var-init=
>> -Wtrivial-auto-var-init
>>
>> for -ftrivial-auto-var-init, we should take the candidate that
>> has a difference in trailing sign symbol.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
>> Thanks,
>> Martin
>>
>> PR driver/105564
>>
>> gcc/ChangeLog:
>>
>> * spellcheck.cc (test_find_closest_string): Add new test.
>> * spellcheck.h (class best_match): Prefer a difference in
>> trailing sign symbol.
>> ---
>> gcc/spellcheck.cc | 9 +++++++++
>> gcc/spellcheck.h | 17 ++++++++++++++---
>> 2 files changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/gcc/spellcheck.cc b/gcc/spellcheck.cc
>> index 3e58344f510..f728573331f 100644
>> --- a/gcc/spellcheck.cc
>> +++ b/gcc/spellcheck.cc
>> @@ -464,6 +464,15 @@ test_find_closest_string ()
>> ASSERT_STREQ ("DWARF_GNAT_ENCODINGS_ALL",
>> find_closest_string ("DWARF_GNAT_ENCODINGS_all",
>> &candidates));
>> +
>> + /* Example from PR 105564 where option name with missing equal
>> + sign should win. */
>> + candidates.truncate (0);
>> + candidates.safe_push ("-Wtrivial-auto-var-init");
>> + candidates.safe_push ("-ftrivial-auto-var-init=");
>> + ASSERT_STREQ ("-ftrivial-auto-var-init=",
>> + find_closest_string ("-ftrivial-auto-var-init",
>> + &candidates));
>> }
>>
>> /* Test data for test_metric_conditions. */
>> diff --git a/gcc/spellcheck.h b/gcc/spellcheck.h
>> index 9b6223695be..9111ea08fc3 100644
>> --- a/gcc/spellcheck.h
>> +++ b/gcc/spellcheck.h
>> @@ -128,11 +128,22 @@ class best_match
>>
>> /* Otherwise, compute the distance and see if the candidate
>> has beaten the previous best value. */
>> + const char *candidate_str = candidate_traits::get_string
>> (candidate);
>> edit_distance_t dist
>> - = get_edit_distance (m_goal, m_goal_len,
>> - candidate_traits::get_string (candidate),
>> - candidate_len);
>> + = get_edit_distance (m_goal, m_goal_len, candidate_str,
>> candidate_len);
>> +
>> + bool is_better = false;
>> if (dist < m_best_distance)
>> + is_better = true;
>> + else if (dist == m_best_distance)
>> + {
>> + /* Prefer a candidate has a difference in trailing sign
>> character. */
>> + if (candidate_str[candidate_len - 1] == '='
>> + && m_goal[m_goal_len - 1] != '=')
>> + is_better = true;
>> + }
>
> Thanks for working on this.
>
> Maybe the comment should read:
>
> /* Prefer a candidate that inserts a trailing '=',
> so that for
> "-ftrivial-auto-var-init"
> we suggest
> "-ftrivial-auto-var-init="
> rather than
> "-Wtrivial-auto-var-init". */
I welcome the comment suggestion.
>
> Is the logic correct? It's comparing the candidate with the goal,
> rather than with the current best.
Yes, that's basically saying that there's a difference in the trailing sign char.
> What if both the candidate and the
> current best both add a trailing equal sign?
Then there's a difference somewhere else, and we would follow edit_distance_t.
>
> I find the array access of the final character suspicious - is there
> any chance that either of the lengths could be zero? I don't think so,
> but maybe we should bulletproof things, and move the "is better"
> comparison to a subroutine?
I think one can't have empty string here.
Sending updated version of the patch where I added the comment.
Ready to be installed?
Thanks,
Martin
>
> Hope this is constructive
> Dave
>
>> +
>> + if (is_better)
>> {
>> m_best_distance = dist;
>> m_best_candidate = candidate;
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-opts-improve-option-suggestion.patch
Type: text/x-patch
Size: 2646 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20220512/6779669c/attachment.bin>
More information about the Gcc-patches
mailing list