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". */ Is the logic correct? It's comparing the candidate with the goal, rather than with the current best. What if both the candidate and the current best both add a trailing equal sign? 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? Hope this is constructive Dave > + > + if (is_better) > { > m_best_distance = dist; > m_best_candidate = candidate;