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 8203E3858C83 for ; Tue, 8 Mar 2022 21:18:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8203E3858C83 Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-42-L1Wg8ikVNgOCBvsGvWjFTQ-1; Tue, 08 Mar 2022 16:18:02 -0500 X-MC-Unique: L1Wg8ikVNgOCBvsGvWjFTQ-1 Received: by mail-qv1-f71.google.com with SMTP id l4-20020a0cc204000000b00435ac16d67cso517388qvh.12 for ; Tue, 08 Mar 2022 13:18:02 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:date:to:cc:subject:in-reply-to:message-id :references:mime-version; bh=MJG3/pt6jaDFZIoPM8qWhoge/A8dTy/+uYGpRP+DyWE=; b=1p84Ypt3wcLrEMu1oWsHMyOmHEkWcqzrHMX0i1NJp3C+n4f2cwOYgvLBr7xseojkM7 Y0GRbWByZJXDciFX/0e5U8eXoD+yyqiBWiMFX1nXhTqblKYmeaNX+dgsx9nZC5IeAiiz jA0Ftd0oN1FBdV1rRXZCVBJhnkusPZe4PkbtAqkON+qGoBFwVfiFkdJ6h2lKMPoa+7gw DA4dNIl2tezalX/BmjXmSO2SOvov1YzvYp0nbUx7pT8Atbq6Dm73mQZd+Fa9/2stQFS2 JC9bd+N4uTEd+Y5FSCEOkYHiO+dj3wXsYM8403wrH6jZ9Qi6m2PsdK1EgwUTCyO0gMzs AlzA== X-Gm-Message-State: AOAM532+81F2QQG4EGKQtuUcPokSPLPutoEHqQOyrh7JZ7hDUfIGxGtX KNJV9wCjZ2cZi75ollqb+Ppc+wSNqHKGUW10NbTXROA++kGw3YmNoTvKpuDnBrtDqxJAvNFxT/w dVJPWvnilLY6HpBclEA== X-Received: by 2002:a05:6214:3004:b0:434:ec44:a4aa with SMTP id ke4-20020a056214300400b00434ec44a4aamr13883292qvb.82.1646774281606; Tue, 08 Mar 2022 13:18:01 -0800 (PST) X-Google-Smtp-Source: ABdhPJxacxmc2vXAGhFDj73JRrYFkjaRD4RODTV8MxZVdYr3KFdrAoTVw2rv7+PJHfJZrQL9pD/qcw== X-Received: by 2002:a05:6214:3004:b0:434:ec44:a4aa with SMTP id ke4-20020a056214300400b00434ec44a4aamr13883279qvb.82.1646774281258; Tue, 08 Mar 2022 13:18:01 -0800 (PST) Received: from [192.168.1.130] (ool-18e40894.dyn.optonline.net. [24.228.8.148]) by smtp.gmail.com with ESMTPSA id s19-20020ac85cd3000000b002de4e165ae0sm36455qta.75.2022.03.08.13.18.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 08 Mar 2022 13:18:00 -0800 (PST) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Tue, 8 Mar 2022 16:17:59 -0500 (EST) To: Jason Merrill cc: Patrick Palka , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] c++: detecting copy-init context during CTAD [PR102137] In-Reply-To: Message-ID: References: <20220304182436.122498-1-ppalka@redhat.com> <0c026689-9136-7d66-cc17-fe4fc4c6e5f6@idea> <6962ed32-1424-07c8-c165-0e9441b93433@idea> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-14.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, 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: Tue, 08 Mar 2022 21:18:07 -0000 On Tue, 8 Mar 2022, Jason Merrill wrote: > On 3/8/22 14:38, Patrick Palka wrote: > > On Tue, 8 Mar 2022, Jason Merrill wrote: > > > > > On 3/8/22 11:36, Patrick Palka wrote: > > > > On Mon, 7 Mar 2022, Jason Merrill wrote: > > > > > > > > > On 3/7/22 10:47, Patrick Palka wrote: > > > > > > On Fri, 4 Mar 2022, Jason Merrill wrote: > > > > > > > > > > > > > On 3/4/22 14:24, Patrick Palka wrote: > > > > > > > > Here we're failing to communicate to cp_finish_decl from > > > > > > > > tsubst_expr > > > > > > > > that we're in a copy-initialization context (via the > > > > > > > > LOOKUP_ONLYCONVERTING > > > > > > > > flag), which causes do_class_deduction to always consider > > > > > > > > explicit > > > > > > > > deduction guides when performing CTAD for a templated variable > > > > > > > > initializer. > > > > > > > > > > > > > > > > We could fix this by passing LOOKUP_ONLYCONVERTING appropriately > > > > > > > > when > > > > > > > > calling cp_finish_decl from tsubst_expr, but it seems > > > > > > > > do_class_deduction > > > > > > > > can determine if we're in a copy-init context by simply > > > > > > > > inspecting > > > > > > > > the > > > > > > > > initializer, and thus render its flags parameter unnecessary, > > > > > > > > which > > > > > > > > is > > > > > > > > what this patch implements. (If we were to fix this in > > > > > > > > tsubst_expr > > > > > > > > instead, I think we'd have to inspect the initializer in the > > > > > > > > same > > > > > > > > way > > > > > > > > in order to detect a copy-init context?) > > > > > > > > > > > > > > Hmm, does this affect conversions as well? > > > > > > > > > > > > > > Looks like it does: > > > > > > > > > > > > > > struct A > > > > > > > { > > > > > > > explicit operator int(); > > > > > > > }; > > > > > > > > > > > > > > template void f() > > > > > > > { > > > > > > > T t = A(); > > > > > > > } > > > > > > > > > > > > > > int main() > > > > > > > { > > > > > > > f(); // wrongly accepted > > > > > > > } > > > > > > > > > > > > > > The reverse, initializing via an explicit constructor, is caught > > > > > > > by > > > > > > > code > > > > > > > in > > > > > > > build_aggr_init much like the code your patch adds to > > > > > > > do_auto_deduction; > > > > > > > perhaps we should move/copy that code to cp_finish_decl? > > > > > > > > > > > > Ah, makes sense. Moving that code from build_aggr_init to > > > > > > cp_finish_decl broke things, but using it in both spots seems to > > > > > > work > > > > > > well. And I suppose we might as well use it in do_class_deduction > > > > > > too, > > > > > > since doing so lets us remove the flags parameter. > > > > > > > > > > Before removing the flags parameter please try asserting that it now > > > > > matches > > > > > is_copy_initialization and see if anything breaks. > > > > > > > > I added to do_class_deduction: > > > > > > > > gcc_assert (bool(flags & LOOKUP_ONLYCONVERTING) == > > > > is_copy_initialization > > > > (init)); > > > > > > > > Turns out removing the flags parameter breaks CTAD for new-expressions > > > > of the form 'new TT(x)' because in this case build_new passes just 'x' > > > > as the initializer to do_auto_deduction (as opposed to a single > > > > TREE_LIST), > > > > for which is_copy_initialization returns true even though it's really > > > > direct initalization. > > > > > > > > Also turns out we're similarly not passing the right LOOKUP_* flags to > > > > cp_finish_decl from instantiate_body, which breaks consideration of > > > > explicit conversions/deduction guides when instantiating the initializer > > > > of a static data member. I added some xfailed testcases for these > > > > situations. > > > > > > Maybe we want to check is_copy_initialization in cp_finish_decl? > > > > That seems to work nicely :) All xfailed tests for the static data > > member initialization case now also pass. > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > > trunk? > > > > -- >8 -- > > > > Subject: [PATCH] c++: detecting copy-init context during CTAD [PR102137] > > > > Here we're failing to communicate to cp_finish_decl from tsubst_expr > > that we're in a copy-initialization context (via the LOOKUP_ONLYCONVERTING > > flag), which causes us to always consider explicit deduction guides when > > performing CTAD for a templated variable initializer. > > > > It turns out this bug also affects consideration of explicit conversion > > operators for the same reason. But consideration of explicit constructors > > seems to do the right thing thanks to code in build_aggr_init that sets > > LOOKUP_ONLYCONVERTING when the initializer represents copy-initialization. > > > > This patch fixes this by making cp_finish_decl set LOOKUP_ONLYCONVERTING > > by inspecting the initializer like build_aggr_init does, so that callers > > don't need to explicitly pass this flag. > > > > PR c++/102137 > > PR c++/87820 > > > > gcc/cp/ChangeLog: > > > > * cp-tree.h (is_copy_initialization): Declare. > > * decl.cc (cp_finish_decl): Set LOOKUP_ONLYCONVERTING > > when is_copy_initialization is true. > > * init.cc (build_aggr_init): Split out copy-initialization > > check into ... > > (is_copy_initialization): ... here. > > * pt.cc (instantiate_decl): Pass 0 instead of > > LOOKUP_ONLYCONVERTING as flags to cp_finish_decl. > > Any reason not to use LOOKUP_NORMAL, both here and in tsubst_expr? OK either > way. Thanks a lot. I'm not really sure what the consequences of using LOOKUP_NORMAL (which implies LOOKUP_PROTECT) instead of 0 would be here, and there's a bunch of other callers of cp_finish_decl which pass 0 or some other value that excludes LOOKUP_PROTECT. So I figure such a change would be better off as a separate patch that changes/audits all such cp_finish_decl at once. > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/cpp0x/explicit15.C: New test. > > * g++.dg/cpp1z/class-deduction108.C: New test. > > --- > > gcc/cp/cp-tree.h | 1 + > > gcc/cp/decl.cc | 3 + > > gcc/cp/init.cc | 20 +++-- > > gcc/cp/pt.cc | 3 +- > > gcc/testsuite/g++.dg/cpp0x/explicit15.C | 83 +++++++++++++++++++ > > .../g++.dg/cpp1z/class-deduction108.C | 78 +++++++++++++++++ > > 6 files changed, 181 insertions(+), 7 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp0x/explicit15.C > > create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction108.C > > > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > > index ac723901098..fd76909ca75 100644 > > --- a/gcc/cp/cp-tree.h > > +++ b/gcc/cp/cp-tree.h > > @@ -7039,6 +7039,7 @@ extern void emit_mem_initializers > > (tree); > > extern tree build_aggr_init (tree, tree, int, > > tsubst_flags_t); > > extern int is_class_type (tree, int); > > +extern bool is_copy_initialization (tree); > > extern tree build_zero_init (tree, tree, bool); > > extern tree build_value_init (tree, > > tsubst_flags_t); > > extern tree build_value_init_noctor (tree, tsubst_flags_t); > > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > > index 199ac768d43..5452f7d9cb3 100644 > > --- a/gcc/cp/decl.cc > > +++ b/gcc/cp/decl.cc > > @@ -7968,6 +7968,9 @@ cp_finish_decl (tree decl, tree init, bool > > init_const_expr_p, > > if (type == error_mark_node) > > return; > > + if (VAR_P (decl) && is_copy_initialization (init)) > > + flags |= LOOKUP_ONLYCONVERTING; > > + > > /* Warn about register storage specifiers except when in GNU global > > or local register variable extension. */ > > if (VAR_P (decl) && DECL_REGISTER (decl) && asmspec_tree == NULL_TREE) > > diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc > > index 545d904c0f9..d5faa0dc519 100644 > > --- a/gcc/cp/init.cc > > +++ b/gcc/cp/init.cc > > @@ -2019,11 +2019,7 @@ build_aggr_init (tree exp, tree init, int flags, > > tsubst_flags_t complain) > > return stmt_expr; > > } > > - if (init && init != void_type_node > > - && TREE_CODE (init) != TREE_LIST > > - && !(TREE_CODE (init) == TARGET_EXPR > > - && TARGET_EXPR_DIRECT_INIT_P (init)) > > - && !DIRECT_LIST_INIT_P (init)) > > + if (is_copy_initialization (init)) > > flags |= LOOKUP_ONLYCONVERTING; > > is_global = begin_init_stmts (&stmt_expr, &compound_stmt); > > @@ -2331,6 +2327,20 @@ is_class_type (tree type, int or_else) > > return 1; > > } > > +/* Returns true iff the variable initializer INIT represents > > + copy-initialization (and therefore we must set LOOKUP_ONLYCONVERTING > > + when processing it). */ > > + > > +bool > > +is_copy_initialization (tree init) > > +{ > > + return (init && init != void_type_node > > + && TREE_CODE (init) != TREE_LIST > > + && !(TREE_CODE (init) == TARGET_EXPR > > + && TARGET_EXPR_DIRECT_INIT_P (init)) > > + && !DIRECT_LIST_INIT_P (init)); > > +} > > + > > /* Build a reference to a member of an aggregate. This is not a C++ > > `&', but really something which can have its address taken, and > > then act as a pointer to member, for example TYPE :: FIELD can have > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > > index 402365285cf..e8b5d8fbb73 100644 > > --- a/gcc/cp/pt.cc > > +++ b/gcc/cp/pt.cc > > @@ -26602,8 +26602,7 @@ instantiate_decl (tree d, bool defer_ok, bool > > expl_inst_class_mem_p) > > const_init > > = DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (code_pattern); > > cp_finish_decl (d, init, /*init_const_expr_p=*/const_init, > > - /*asmspec_tree=*/NULL_TREE, > > - LOOKUP_ONLYCONVERTING); > > + /*asmspec_tree=*/NULL_TREE, 0); > > } > > if (enter_context) > > pop_nested_class (); > > diff --git a/gcc/testsuite/g++.dg/cpp0x/explicit15.C > > b/gcc/testsuite/g++.dg/cpp0x/explicit15.C > > new file mode 100644 > > index 00000000000..076d8b31631 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp0x/explicit15.C > > @@ -0,0 +1,83 @@ > > +// PR c++/87820 > > +// { dg-do compile { target c++11 } } > > + > > +struct A { > > + constexpr explicit operator int() const { return 0; } > > +}; > > + > > +template > > +void f() { > > + A a; > > + T t1 = a; // { dg-error "cannot convert" } > > + T t2 = {a}; // { dg-error "cannot convert" } > > + T t3(a); > > + T t4{a}; > > + T t5 = T(a); > > + T t6 = T{a}; > > + new T(a); > > + new T{a}; > > +} > > + > > +template > > +void g() { > > + T t; > > + int n1 = t; // { dg-error "cannot convert" } > > + int n2 = {t}; // { dg-error "cannot convert" } > > + int n3(t); > > + int n4{t}; > > + int n5 = int(t); > > + int n6 = int{t}; > > + new int(t); > > + new int{t}; > > +} > > + > > +template void f(); > > +template void g(); > > + > > +template > > +struct B { > > + static constexpr A a{}; > > + static constexpr T t1 = a; // { dg-error "cannot convert" } > > + static constexpr T t2 = {a}; // { dg-error "cannot convert" } > > + static constexpr T t4{a}; // { dg-bogus "cannot convert" } > > + static constexpr T t5 = T(a); > > + static constexpr T t6 = T{a}; > > +}; > > + > > +template > > +struct C { > > + static constexpr T t{}; > > + static constexpr int n1 = t; // { dg-error "cannot convert" } > > + static constexpr int n2 = {t}; // { dg-error "cannot convert" } > > + static constexpr int n4{t}; // { dg-bogus "cannot convert" } > > + static constexpr int n5 = int(t); > > + static constexpr int n6 = int{t}; > > +}; > > + > > +template struct B; > > +template struct C; > > + > > +#if __cpp_inline_variables > > +template > > +struct D { > > + static inline A a; > > + static inline T t1 = a; // { dg-error "cannot convert" "" { target c++17 > > } } > > + static inline T t2 = {a}; // { dg-error "cannot convert" "" { target > > c++17 } } > > + static inline T t4{a}; > > + static inline T t5 = T(a); > > + static inline T t6 = T{a}; > > +}; > > + > > +template > > +struct E { > > + static inline T t; > > + static inline int n1 = t; // { dg-error "cannot convert" "" { target > > c++17 } } > > + static inline int n2 = {t}; // { dg-error "cannot convert" "" { target > > c++17 } } > > + static inline int n4{t}; > > + static inline int n5 = int(t); > > + static inline int n6 = int{t}; > > +}; > > + > > +template struct D; > > +template struct E; > > +#endif > > diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction108.C > > b/gcc/testsuite/g++.dg/cpp1z/class-deduction108.C > > new file mode 100644 > > index 00000000000..e82c4bafb04 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction108.C > > @@ -0,0 +1,78 @@ > > +// PR c++/102137 > > +// { dg-do compile { target c++17 } } > > + > > +template > > +struct A { > > + constexpr A() { } > > + constexpr A(int) { } > > +}; > > + > > +explicit A(...) -> A; > > + > > +template class TT> > > +void f() { > > + TT x1 = 0; // { dg-error "deduction|no match" } > > + TT x2 = {0}; // { dg-error "explicit deduction guide" } > > + TT x3(0); > > + TT x4{0}; > > + TT x5; > > + new TT(0); > > + new TT{0}; > > + new TT(); > > + new TT{}; > > + new TT; > > +} > > + > > +template > > +void g(T t) { > > + A a1 = t; // { dg-error "deduction|no match" } > > + A a2 = {t}; // { dg-error "explicit deduction guide" } > > + A a3(t); > > + A a4{t}; > > + A a5; > > + new A(t); > > + new A{t}; > > +} > > + > > +template void f(); > > +template void g(int); > > + > > +template class TT> > > +struct B { > > + static inline TT x1 = 0; // { dg-error "deduction|no match" } > > + static inline TT x2 = {0}; // { dg-error "explicit deduction guide" } > > + static inline TT x4{0}; > > + static inline TT x5; > > +}; > > + > > +template > > +struct C { > > + static inline T t; > > + static inline A a1 = t; // { dg-error "deduction|no match" } > > + static inline A a2 = {t}; // { dg-error "explicit deduction guide" } > > + static inline A a4{t}; > > + static inline A a5{}; > > +}; > > + > > +template struct B; > > +template struct C; > > + > > +template class TT> > > +struct E { > > + static constexpr TT x1 = 0; // { dg-error "deduction|no match" } > > + static constexpr TT x2 = {0}; // { dg-error "explicit deduction guide" } > > + static constexpr TT x4{0}; > > + static constexpr TT x5{}; > > +}; > > + > > +template > > +struct F { > > + static constexpr T t{}; > > + static constexpr A a1 = t; // { dg-error "deduction|no match" } > > + static constexpr A a2 = {t}; // { dg-error "explicit deduction guide" } > > + static constexpr A a4{t}; > > + static constexpr A a5{}; > > +}; > > + > > +template struct E; > > +template struct F; > >