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 D9A283851A85 for ; Mon, 18 Jul 2022 20:44:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D9A283851A85 Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-106-l6MP7aoPPQSRCU__77cOCQ-1; Mon, 18 Jul 2022 16:44:12 -0400 X-MC-Unique: l6MP7aoPPQSRCU__77cOCQ-1 Received: by mail-qk1-f199.google.com with SMTP id s9-20020a05620a254900b006b54dd4d6deso10107403qko.3 for ; Mon, 18 Jul 2022 13:44:12 -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=bHJSdqscBowZW06mJXZFSnjN51f6dEek0/8m02oNlPE=; b=M8wOMM02UgxofUeIYHfsYEpU+2+LK+5ImdeIEKcYEPvA2srBNHvb/8sX176o6V3Gzd M3CzLY11iGQG10bMQQVO+v0U2+SWwyLgCHrXrMvmsrHQ0UX1/2FQiX7fP3QS82/559BP lFmv26Uv2GY1XlatFw+7twh2bQeBg6brkftb6otpuTzf9qe2hAi/BJAdmv3X5QN2E5Uu FMUkYhs/UHdqXFi4x249Q5rmKSTxqQo0JCCwpN8tcIorRfKWoD20MsGPVbBMKKeosDBw eR5kX5Dlu+chSGhyVG6mOQbWoCvXz0iXdle6V/3vXQA++iEBDuGV49rIVWxRUDVOr+3/ +kLQ== X-Gm-Message-State: AJIora87FEA9DK7A8pDbtIGryGVCZ9U09S5vTAh0T860mI4Uour4HH76 RonblXqnOyiI5aNEGPszgzv0UkNUSerbMpq1sgTFVYvNiU9k08GXAB9Yl5l7lRyndCLQgOwEQ9f xTuGVkwYlXiwICxTdJw== X-Received: by 2002:a05:620a:22fa:b0:6af:6c8c:32e8 with SMTP id p26-20020a05620a22fa00b006af6c8c32e8mr18996906qki.15.1658177051858; Mon, 18 Jul 2022 13:44:11 -0700 (PDT) X-Google-Smtp-Source: AGRyM1vZdTZ6Ggy0vuz/erM+4xhLu6tDlUBkk2ocq5yZtVpxP7gtb1kcBMLofdFhZukrmEyXXUy6yA== X-Received: by 2002:a05:620a:22fa:b0:6af:6c8c:32e8 with SMTP id p26-20020a05620a22fa00b006af6c8c32e8mr18996887qki.15.1658177051315; Mon, 18 Jul 2022 13:44:11 -0700 (PDT) Received: from [192.168.1.100] (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id bk34-20020a05620a1a2200b006af1f0af045sm11827254qkb.107.2022.07.18.13.44.10 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 18 Jul 2022 13:44:10 -0700 (PDT) Message-ID: <33107706-023b-d00a-69b0-93dc39889534@redhat.com> Date: Mon, 18 Jul 2022 16:44:09 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH] c++: shortcut bad reference bindings [PR94894] To: Patrick Palka , gcc-patches@gcc.gnu.org References: <20220718165911.3559847-1-ppalka@redhat.com> From: Jason Merrill In-Reply-To: <20220718165911.3559847-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=-12.7 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_NONE, SPF_HELO_NONE, SPF_NONE, TXREP 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: Mon, 18 Jul 2022 20:44:15 -0000 On 7/18/22 12:59, Patrick Palka wrote: > In case of l/rvalue or cv-qual mismatch during reference binding, we try > to give more helpful diagnostics by attempting a bad conversion that > ignores the mismatch. But in doing so, we may end up instantiating an > ill-formed conversion function, something that would otherwise be > avoided if we didn't try for the better diagnostic in the first place. > We could just give up on producing a good diagnostic in this case, but > ideally we could keep the good diagnostics while avoiding unnecessary > template instantiations. > > To that end, this patch adapts the bad conversion shortcutting mechanism > from r12-3346-g47543e5f9d1fc5 to handle this situation as well. The main > observation from there applies here as well: when there's a strictly > viable candidate, we don't care about the distinction between an unviable > and non-strictly viable candidate. And in turn it also means we don't > really care about the distinction between an invalid and bad conversion. > Of course, we don't know whether there's a strictly viable candidate until > after the fact, so we still need to keep track of when we "shortcutted" > distinguishing between an invalid and bad conversion. This patch adds a > new kind of conversion, ck_shortcutted, to represent such conversions. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk? > > PR c++/94894 > PR c++/105766 > PR c++/106201 > > gcc/cp/ChangeLog: > > * call.cc (enum conversion_kind): Add ck_shortcutted enumerator. > (has_next): Return false for it. > (reference_binding): Return a ck_shortcutted conversion instead > of an actual bad conversion when LOOKUP_SHORTCUT_BAD_CONVS is set. > Remove now obsolete early exit for the incomplete target type case. > (implicit_conversion_1): Don't mask out LOOKUP_SHORTCUT_BAD_CONVS. > (add_function_candidate): Set LOOKUP_SHORTCUT_BAD_CONVS iff > shortcut_bad_convs. > (missing_conversion_p): Return true for a ck_shortcutted > conversion. > * cp-tree.h (LOOKUP_SHORTCUT_BAD_CONVS): Define. > > gcc/testsuite/ChangeLog: > > * g++.dg/conversion/ref8.C: New test. > * g++.dg/conversion/ref9.C: New test. > --- > gcc/cp/call.cc | 87 ++++++++++++++++---------- > gcc/cp/cp-tree.h | 5 ++ > gcc/testsuite/g++.dg/conversion/ref8.C | 22 +++++++ > gcc/testsuite/g++.dg/conversion/ref9.C | 21 +++++++ > 4 files changed, 103 insertions(+), 32 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/conversion/ref8.C > create mode 100644 gcc/testsuite/g++.dg/conversion/ref9.C > > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc > index fc98552fda2..ca2f5fbca39 100644 > --- a/gcc/cp/call.cc > +++ b/gcc/cp/call.cc > @@ -59,7 +59,8 @@ enum conversion_kind { > ck_ambig, > ck_list, > ck_aggr, > - ck_rvalue > + ck_rvalue, > + ck_shortcutted, Please give this a comment explaining how it's used. OK with that change. Maybe ck_deferred_bad? > }; > > /* The rank of the conversion. Order of the enumerals matters; better > @@ -775,7 +776,8 @@ has_next (conversion_kind code) > return !(code == ck_identity > || code == ck_ambig > || code == ck_list > - || code == ck_aggr); > + || code == ck_aggr > + || code == ck_shortcutted); > } > > static conversion * > @@ -1912,18 +1914,38 @@ reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags, > difference in top-level cv-qualification is subsumed by the > initialization itself and does not constitute a conversion. */ > > + bool maybe_valid_p = true; > + > /* [dcl.init.ref] > > Otherwise, the reference shall be an lvalue reference to a > non-volatile const type, or the reference shall be an rvalue > - reference. > + reference. */ > + if (!CP_TYPE_CONST_NON_VOLATILE_P (to) && !TYPE_REF_IS_RVALUE (rto)) > + maybe_valid_p = false; > > - We try below to treat this as a bad conversion to improve diagnostics, > - but if TO is an incomplete class, we need to reject this conversion > - now to avoid unnecessary instantiation. */ > - if (!CP_TYPE_CONST_NON_VOLATILE_P (to) && !TYPE_REF_IS_RVALUE (rto) > - && !COMPLETE_TYPE_P (to)) > - return NULL; > + /* [dcl.init.ref] > + > + Otherwise, a temporary of type "cv1 T1" is created and > + initialized from the initializer expression using the rules for a > + non-reference copy initialization. If T1 is reference-related to > + T2, cv1 must be the same cv-qualification as, or greater > + cv-qualification than, cv2; otherwise, the program is ill-formed. */ > + if (related_p && !at_least_as_qualified_p (to, from)) > + maybe_valid_p = false; > + > + /* We try below to treat an invalid reference binding as a bad conversion > + to improve diagnostics, but doing so may cause otherwise unnecessary > + instantiations that can lead to hard error. So during the first pass > + of overload resolution wherein we shortcut bad conversions, instead just > + produce a special conversion that we'll use to indicate we might need to > + perform a second pass if there's no strictly viable candidate. */ > + if (!maybe_valid_p && (flags & LOOKUP_SHORTCUT_BAD_CONVS)) > + { > + conv = alloc_conversion (ck_shortcutted); > + conv->bad_p = true; > + return conv; > + } > > /* We're generating a temporary now, but don't bind any more in the > conversion (specifically, don't slice the temporary returned by a > @@ -1967,7 +1989,9 @@ reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags, > sflags, complain); > if (!new_second) > return NULL; > - return merge_conversion_sequences (t, new_second); > + conv = merge_conversion_sequences (t, new_second); > + gcc_assert (maybe_valid_p || conv->bad_p); > + return conv; > } > } > > @@ -1976,24 +2000,7 @@ reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags, > creation of a temporary. */ > conv->need_temporary_p = true; > conv->rvaluedness_matches_p = TYPE_REF_IS_RVALUE (rto); > - > - /* [dcl.init.ref] > - > - Otherwise, the reference shall be an lvalue reference to a > - non-volatile const type, or the reference shall be an rvalue > - reference. */ > - if (!CP_TYPE_CONST_NON_VOLATILE_P (to) && !TYPE_REF_IS_RVALUE (rto)) > - conv->bad_p = true; > - > - /* [dcl.init.ref] > - > - Otherwise, a temporary of type "cv1 T1" is created and > - initialized from the initializer expression using the rules for a > - non-reference copy initialization. If T1 is reference-related to > - T2, cv1 must be the same cv-qualification as, or greater > - cv-qualification than, cv2; otherwise, the program is ill-formed. */ > - if (related_p && !at_least_as_qualified_p (to, from)) > - conv->bad_p = true; > + conv->bad_p |= !maybe_valid_p; > > return conv; > } > @@ -2015,7 +2022,8 @@ implicit_conversion_1 (tree to, tree from, tree expr, bool c_cast_p, > resolution, or after we've chosen one. */ > flags &= (LOOKUP_ONLYCONVERTING|LOOKUP_NO_CONVERSION|LOOKUP_COPY_PARM > |LOOKUP_NO_TEMP_BIND|LOOKUP_NO_RVAL_BIND|LOOKUP_PREFER_RVALUE > - |LOOKUP_NO_NARROWING|LOOKUP_PROTECT|LOOKUP_NO_NON_INTEGRAL); > + |LOOKUP_NO_NARROWING|LOOKUP_PROTECT|LOOKUP_NO_NON_INTEGRAL > + |LOOKUP_SHORTCUT_BAD_CONVS); > > /* FIXME: actually we don't want warnings either, but we can't just > have 'complain &= ~(tf_warning|tf_error)' because it would cause > @@ -2433,6 +2441,11 @@ add_function_candidate (struct z_candidate **candidates, > if (! viable) > goto out; > > + if (shortcut_bad_convs) > + flags |= LOOKUP_SHORTCUT_BAD_CONVS; > + else > + flags &= ~LOOKUP_SHORTCUT_BAD_CONVS; > + > /* Third, for F to be a viable function, there shall exist for each > argument an implicit conversion sequence that converts that argument > to the corresponding parameter of F. */ > @@ -6038,14 +6051,24 @@ perfect_candidate_p (z_candidate *cand) > return true; > } > > -/* True iff one of CAND's argument conversions is NULL. */ > +/* True iff one of CAND's argument conversions is missing. */ > > static bool > missing_conversion_p (const z_candidate *cand) > { > for (unsigned i = 0; i < cand->num_convs; ++i) > - if (!cand->convs[i]) > - return true; > + { > + conversion *conv = cand->convs[i]; > + if (!conv) > + return true; > + if (conv->kind == ck_shortcutted) > + { > + /* We don't know whether this conversion is outright invalid or > + just bad, so be conservative. */ > + gcc_checking_assert (conv->bad_p); > + return true; > + } > + } > return false; > } > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index bec98aa2ac3..f3b7da2549c 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -5874,6 +5874,11 @@ enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG, TYPENAME_FLAG }; > #define LOOKUP_REVERSED (LOOKUP_REWRITTEN << 1) > /* We're initializing an aggregate from a parenthesized list of values. */ > #define LOOKUP_AGGREGATE_PAREN_INIT (LOOKUP_REVERSED << 1) > +/* We're computing conversions as part of a first pass of overload resolution > + wherein we don't try to distinguish an unviable candidate from a > + non-strictly viable candidate and thus can avoid computing unnecessary > + conversions. */ > +#define LOOKUP_SHORTCUT_BAD_CONVS (LOOKUP_AGGREGATE_PAREN_INIT << 1) > > /* These flags are used by the conversion code. > CONV_IMPLICIT : Perform implicit conversions (standard and user-defined). > diff --git a/gcc/testsuite/g++.dg/conversion/ref8.C b/gcc/testsuite/g++.dg/conversion/ref8.C > new file mode 100644 > index 00000000000..e27f642690c > --- /dev/null > +++ b/gcc/testsuite/g++.dg/conversion/ref8.C > @@ -0,0 +1,22 @@ > +// PR c++/105766 > +// { dg-do compile { target c++20 } } > + > +template > +struct baz { > + baz() = default; > + baz(int) requires __is_constructible(T, int); > +}; > + > +struct foo; > + > +struct bar { > + bar() = default; > + bar(foo&); > + bar(int); > +}; > + > +struct foo { > + baz m_bars; > +}; > + > +foo a; > diff --git a/gcc/testsuite/g++.dg/conversion/ref9.C b/gcc/testsuite/g++.dg/conversion/ref9.C > new file mode 100644 > index 00000000000..e6dfc03cd7f > --- /dev/null > +++ b/gcc/testsuite/g++.dg/conversion/ref9.C > @@ -0,0 +1,21 @@ > +// PR c++/106201 > +// { dg-do compile { target c++11 } } > + > +struct A { > + template > + A(const T&); > +}; > + > +struct B { > + template B(const T&); > +}; > + > +void f(A&); > +void f(B); > + > +struct C { }; > + > +int main() { > + C c; > + f(c); > +}