From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 4643D3853809 for ; Wed, 11 May 2022 18:49:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4643D3853809 Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-650-YncKk2QfPbuv6HOS4H1yHQ-1; Wed, 11 May 2022 14:49:20 -0400 X-MC-Unique: YncKk2QfPbuv6HOS4H1yHQ-1 Received: by mail-qv1-f70.google.com with SMTP id m6-20020a0562141bc600b0045a8357224aso2677705qvc.12 for ; Wed, 11 May 2022 11:49:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:subject:from:to:date:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=9WkJa/aAAlzTXuZyUt01PouX/oX1qg85bE+M+0Q5QYA=; b=v5XJQ/PPUfOi7LryQA8e8YfViQVIHwE4GG7jgyxfI2TOGFdmk3ysfOroOKPuIuKLd/ SX9cTZ65R+tXQPN+ihZodBNJW+xAZvDuj6CDnPM1aOic3nLBeiU5dTywN44jkJcDCw0Y NwkbM1hiczalPuCbdAdIzpLUtHaoX1lYNNQnsYcMM8zFdE8p0Zsf+ZaLvuR6gTW1/nfi XN4uj329fHDbpmSWBz2UNpbzWh+6DewJHb64Yx4Ybf6ab1z7Tho34fRIIqKWm3gk8koB ppqT07X+59ViCyT1Q84de2dRiSJh7lsiYl23BUJn7qi6OYVw2OkJRev1ngLTWA0CNAqT z7Pw== X-Gm-Message-State: AOAM533HEM4pJSzeS/68fi1t49tL4MCVq4ofc/Olp2qSCViwGweeClOK djnBZPexYOyZhc4znTpojQFniqDpKhUwhDaWRh9kmLAVSHJsR89pLsuZjsekPeoPGk6yzjcNPod TIyUMIUlBd7yMWk+/Bw== X-Received: by 2002:ac8:5896:0:b0:2f3:d231:58a9 with SMTP id t22-20020ac85896000000b002f3d23158a9mr19709897qta.131.1652294960064; Wed, 11 May 2022 11:49:20 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz1pKOTvElhKLyQXutk2vuJBGXpUWEgzeIciQv/PFoclrEpmyKozKk6xwFZzJKQwwzwICKy+Q== X-Received: by 2002:ac8:5896:0:b0:2f3:d231:58a9 with SMTP id t22-20020ac85896000000b002f3d23158a9mr19709881qta.131.1652294959794; Wed, 11 May 2022 11:49:19 -0700 (PDT) Received: from t14s.localdomain (c-73-69-212-193.hsd1.nh.comcast.net. [73.69.212.193]) by smtp.gmail.com with ESMTPSA id t4-20020ae9df04000000b006a09faf5c1asm1680916qkf.30.2022.05.11.11.49.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 May 2022 11:49:19 -0700 (PDT) Message-ID: <3b3b807f1dca4aa46e3068973fa301327806014a.camel@redhat.com> Subject: Re: [PATCH] opts: improve option suggestion From: David Malcolm To: Martin =?UTF-8?Q?Li=C5=A1ka?= , gcc-patches@gcc.gnu.org Date: Wed, 11 May 2022 14:49:17 -0400 In-Reply-To: References: User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Wed, 11 May 2022 18:49:24 -0000 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;