From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vs1-xe2a.google.com (mail-vs1-xe2a.google.com [IPv6:2607:f8b0:4864:20::e2a]) by sourceware.org (Postfix) with ESMTPS id EA1103857427 for ; Thu, 12 May 2022 07:05:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EA1103857427 Received: by mail-vs1-xe2a.google.com with SMTP id u205so4249642vsu.6 for ; Thu, 12 May 2022 00:05:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=Rjsm69hq01jzbYgJV/VwbV8HmoBa0xrFzfwgTi+io4g=; b=l/RHcXZK8ZANxvhxHU0gAJxdVZKzse3EonGfOnQa9bMGiP9OWYGRgmGan0BsHHgrIb 2J2REQ9fQG+pN0NOTmmTycYa6PXl4UIRFabKOi3fbExYMvCRnOBCa6RmWyRWG6lkoyZ2 aTlUlRjhHThIeaFDJ/e33DgSwmBHGdpJ026U0AoxiWWA9HedyN2tdRM4v5oUk7gu5tT9 2XrDgBSGARq/L+LdEyrS0uLxXkBO213tcmyZdZQtpJnWaVa7jk6wK1PnHpm6hiZIZCPs 0yWgCl1XA0xUHhB79ma3Vk/gdz8FLRqyDZ9G3q8PjwvFoDtRE5TSS+wKoaGEnqlWCvLE 6Djw== X-Gm-Message-State: AOAM531lqevb2sFINwqvwKftAJoxXh7upc4A+F1Z4kmD5NU/L0tXtm9o DlY76LA5tADWU+8Aa5webyf+YUHrQxrg1i/PFAM= X-Google-Smtp-Source: ABdhPJztokXkIKR7f2KUWnW8ZFHtmGzLZ5Gyf/cK+6cAV0WtVXV/I3z+DekMvvCWSZxPr8uuA9zqyRrSkLT8dc96Mss= X-Received: by 2002:a05:6102:320c:b0:32c:ffc1:ff1c with SMTP id r12-20020a056102320c00b0032cffc1ff1cmr14968249vsf.35.1652339123318; Thu, 12 May 2022 00:05:23 -0700 (PDT) MIME-Version: 1.0 References: <3b3b807f1dca4aa46e3068973fa301327806014a.camel@redhat.com> In-Reply-To: <3b3b807f1dca4aa46e3068973fa301327806014a.camel@redhat.com> From: Richard Biener Date: Thu, 12 May 2022 09:05:11 +0200 Message-ID: Subject: Re: [PATCH] opts: improve option suggestion To: David Malcolm Cc: =?UTF-8?Q?Martin_Li=C5=A1ka?= , GCC Patches Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 May 2022 07:05:25 -0000 On Wed, May 11, 2022 at 8:50 PM David Malcolm via Gcc-patches wrote: > > On Wed, 2022-05-11 at 16:49 +0200, Martin Li=C5=A1ka wrote: > > In case where we have 2 equally good candidates like > > -ftrivial-auto-var-init=3D > > -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=3D"); > > + ASSERT_STREQ ("-ftrivial-auto-var-init=3D", > > + 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 =3D candidate_traits::get_string > > (candidate); > > edit_distance_t dist > > - =3D get_edit_distance (m_goal, m_goal_len, > > - candidate_traits::get_string (candidate), > > - candidate_len); > > + =3D get_edit_distance (m_goal, m_goal_len, candidate_str, > > candidate_len); > > + > > + bool is_better =3D false; > > if (dist < m_best_distance) > > + is_better =3D true; > > + else if (dist =3D=3D m_best_distance) > > + { > > + /* Prefer a candidate has a difference in trailing sign > > character. */ > > + if (candidate_str[candidate_len - 1] =3D=3D '=3D' > > + && m_goal[m_goal_len - 1] !=3D '=3D') > > + is_better =3D true; > > + } > > Thanks for working on this. > > Maybe the comment should read: > > /* Prefer a candidate that inserts a trailing '=3D', > so that for > "-ftrivial-auto-var-init" > we suggest > "-ftrivial-auto-var-init=3D" > 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? I think for this case the logic would be to prefer to stick to the -W or -f _prefix_ the user gave and give that a "boost" so we do not suggest an optimization option for a warning one. That is, when given -Wfoo we should not suggest -ffoo-bar but -Wfoo-longbar (just made up example), even if the -ffoo-bar looks a much better match. But maybe that's better handled by better selecting the initial candidate set rather than doing the last disambiguation. Richard. > Hope this is constructive > Dave > > > + > > + if (is_better) > > { > > m_best_distance =3D dist; > > m_best_candidate =3D candidate; > >