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 [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 71D2B3858403 for ; Wed, 15 Sep 2021 17:36:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 71D2B3858403 Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-591-r7YptlFuP2y56dRvech9Ng-1; Wed, 15 Sep 2021 13:36:52 -0400 X-MC-Unique: r7YptlFuP2y56dRvech9Ng-1 Received: by mail-qv1-f69.google.com with SMTP id u6-20020ad449a6000000b003798010ad14so6341296qvx.10 for ; Wed, 15 Sep 2021 10:36:52 -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:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=tt+OVs108ihHdP7jvXn+WTa+g+3w9idKlfs+YrS86i0=; b=1NsHq9u2FURtMvH/cBG9f6lPkZrhSfSEYGgHWaXbDW8hcsp9qEhaROy7BtLfH/eOYR hZR0JyYzluS1skScPRJyGXVdSe6xeQo64wJB4JrtMtV7268gofDyZcc+9oreM02ZvNzF 8QAdcgikCum9sqTorjk1eKrhab+h1AafO9Uqs4Ca62xG45u6SuDdFti4sC0Os0xfqViR CcXEAW+ZbA+OnUsR5ANGwiCasCsh1/uOgaiNRT9FIJkkQStF22Z2WysA8KSqXhYprkQT MvnYBN71X9hoefJYq99zTgjaJz2q1XuMfEYRwr+c5cNHJ1Dt/VaAK1PNS3upyhmR6CaP lLwQ== X-Gm-Message-State: AOAM532fOtCQY/HQU7pVZms9xZIDPxEz196l2SDqnNdj5nsF2syQ+r7B qv827xSdCCIg9CWP/jL0MiQtzFHZIvEFDukdN8RGP7yFrNeipQJlsFu413uOp8XeVkEu8LRHHVu sAw59NByUwyA0xUd4og== X-Received: by 2002:a05:620a:444d:: with SMTP id w13mr1104254qkp.315.1631727411354; Wed, 15 Sep 2021 10:36:51 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx+f0s3AKuO6NCGMjPH4rCHJemWJSsZRiCEtCSCY5IY5T8bRZPdMqqB34EHHJ/Pqu8xxptDMg== X-Received: by 2002:a05:620a:444d:: with SMTP id w13mr1104236qkp.315.1631727411087; Wed, 15 Sep 2021 10:36:51 -0700 (PDT) Received: from [192.168.1.149] (130-44-159-43.s11817.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id n11sm374167qtx.45.2021.09.15.10.36.49 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 15 Sep 2021 10:36:50 -0700 (PDT) Message-ID: <32b05f7c-feb1-39dd-a172-ec2676d3a41c@redhat.com> Date: Wed, 15 Sep 2021 13:36:49 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.0 Subject: Re: [PATCH] c++: shortcut bad convs during overload resolution, part 2 [PR101904] To: Patrick Palka , gcc-patches@gcc.gnu.org References: <20210912194549.2699793-1-ppalka@redhat.com> From: Jason Merrill In-Reply-To: <20210912194549.2699793-1-ppalka@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-13.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP 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, 15 Sep 2021 17:36:55 -0000 On 9/12/21 15:45, Patrick Palka wrote: > The r12-3346 patch makes us avoid computing excess argument conversions > during overload resolution, but only when it turns out there's a > strictly viable candidate in the overload set. If there is no such > candidate then we still need to compute more conversions than strictly > necessary because subsequent conversions after the first bad conversion > can turn a non-strictly viable candidate into unviable one, and that > affects the outcome of overload resolution and the behavior of its > callers (in light of -fpermissive). > > But at least in a SFINAE context, the distinction between a non-strictly > viable and an unviable candidate shouldn't matter all that much since > performing a bad conversion is always an error (even with -fpermissive), > and so forming a call to a non-strictly viable candidate will end up > being a SFINAE error anyway, just like in the unviable case. Hence a > non-strictly viable candidate is effectively unviable (in a SFINAE > context), and we don't really need to distinguish between the two kinds. > We can take advantage of this observation to avoid computing excess > argument conversions even when there's no strictly viable candidate in > the overload set. > > This patch implements this idea. We usually detect a SFINAE context by > looking for the absence of the tf_error flag, but that's not specific > enough: we can also get here from build_user_type_conversion with > tf_error cleared, and there the distinction between a non-strictly > viable candidate and an unviable candidate still matters (it determines > whether a user-defined conversion is bad or just doesn't exist). So this > patch sets and checks for the tf_conv flag to detect this situation too, > which avoids regressing conv2.C below. > > Unlike the previous patch, this one does change the outcome of overload > resolution, but it should do so only in a way that preserves backwards > compatibility with -fpermissive. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK > for trunk? OK. > PR c++/101904 > > gcc/cp/ChangeLog: > > * call.c (build_user_type_conversion_1): Add tf_conv to complain. > (add_candidates): When in a SFINAE context, instead of adding a > candidate to bad_fns just mark it unviable. > > gcc/testsuite/ChangeLog: > > * g++.dg/ext/conv2.C: New test. > * g++.dg/template/conv17.C: Augment test. > --- > gcc/cp/call.c | 17 +++++++++++++++-- > gcc/testsuite/g++.dg/ext/conv2.C | 13 +++++++++++++ > gcc/testsuite/g++.dg/template/conv17.C | 7 +++++++ > 3 files changed, 35 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/ext/conv2.C > > diff --git a/gcc/cp/call.c b/gcc/cp/call.c > index b6011c1a282..ab0d118e34e 100644 > --- a/gcc/cp/call.c > +++ b/gcc/cp/call.c > @@ -4175,6 +4175,9 @@ build_user_type_conversion_1 (tree totype, tree expr, int flags, > flags |= LOOKUP_NO_CONVERSION; > if (BRACE_ENCLOSED_INITIALIZER_P (expr)) > flags |= LOOKUP_NO_NARROWING; > + /* Prevent add_candidates from treating a non-strictly viable candidate > + as an unviable one. */ > + complain |= tf_conv; > > /* It's OK to bind a temporary for converting constructor arguments, but > not in converting the return value of a conversion operator. */ > @@ -6232,8 +6235,18 @@ add_candidates (tree fns, tree first_arg, const vec *args, > stopped at the first bad conversion). Add the function to BAD_FNS > to fully reconsider later if we don't find any strictly viable > candidates. */ > - bad_fns = lookup_add (fn, bad_fns); > - *candidates = (*candidates)->next; > + if (complain & (tf_error | tf_conv)) > + { > + bad_fns = lookup_add (fn, bad_fns); > + *candidates = (*candidates)->next; > + } > + else > + /* But if we're in a SFINAE context, just mark this candidate as > + unviable outright and avoid potentially reconsidering it. > + This is safe to do since performing a bad conversion is always > + erroneous in a SFINAE context (even with -fpermissive), so a > + non-strictly viable candidate is effectively unviable anyway. */ > + cand->viable = 0; > } > } > if (which == non_templates && !seen_perfect) > diff --git a/gcc/testsuite/g++.dg/ext/conv2.C b/gcc/testsuite/g++.dg/ext/conv2.C > new file mode 100644 > index 00000000000..baf2a43b2ae > --- /dev/null > +++ b/gcc/testsuite/g++.dg/ext/conv2.C > @@ -0,0 +1,13 @@ > +// { dg-do compile { target c++11 } } > +// { dg-additional-options "-fpermissive" } > + > +struct A { > + A(int*, int); > +}; > + > +void f(A); > + > +int main() { > + const int n = 0; > + f({&n, 42}); // { dg-warning "invalid conversion from 'const int\\*' to 'int\\*'" } > +} > diff --git a/gcc/testsuite/g++.dg/template/conv17.C b/gcc/testsuite/g++.dg/template/conv17.C > index ba012c9d1fa..e40da8f1f24 100644 > --- a/gcc/testsuite/g++.dg/template/conv17.C > +++ b/gcc/testsuite/g++.dg/template/conv17.C > @@ -53,4 +53,11 @@ concept D = requires (const T t) { > }; > > static_assert(D); > + > +// Test that when there's no strictly viable candidate and we're in a > +// SFINAE context, we still stop at the first bad argument conversion. > +template > +concept E = requires (T t) { T().h(nullptr); }; > + > +static_assert(!E); > #endif >