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 F1AFA3858C74 for ; Tue, 8 Mar 2022 22:26:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org F1AFA3858C74 Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-38-qB7imN4FPEGCriPtuQOiBA-1; Tue, 08 Mar 2022 17:26:19 -0500 X-MC-Unique: qB7imN4FPEGCriPtuQOiBA-1 Received: by mail-qk1-f198.google.com with SMTP id v16-20020a376110000000b0067b2749e0fbso303467qkb.0 for ; Tue, 08 Mar 2022 14:26:19 -0800 (PST) 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:cc:references:from:in-reply-to :content-transfer-encoding; bh=cMW0OGMePqCw5VM/54RJuMrFz7vgVKQ2pwLYXNGNo1o=; b=3xpCtgHiwOTv+Y/7m/78PzBEUe/Cagop+AJxFl/RIggcCibO2ezfnZDatix/B6Cwhw XtqCRUvWymRw46IEN6MCPQQLA1tMgfAN9M0cwiCkrVcKyWzkj7BDxoViIY+9DySl4lJQ sTxal6WQjSDU+aiX5Q1EDKe68pMjKzCYLLTP/diCCQt47M2GBNSz7H8QRB6y1roOaZyg BigU7Q/h4IjruhAdZdcr+ozD05dCUx4LHvADO5IccJtik3lO7fMFWATCL/IQ5n9NGUk7 J26W0Yxjp1bzuxtQ8iwVQDxIkhoq8CGfeouWShzK2dlZg/quXvxY1y4W6vXvwTRkU0fK y6Fw== X-Gm-Message-State: AOAM532yhHFmIjeB10yVZtrmsMIt+A5teQ5GSa9zvBjeDoBJyR+GL/+S 680F9RrOpXMRFT2dBVSSAJ1vPdJWUOujMWpE5g7WLHnZIT4y2/r/lPSeF7ScVsaYLmBDUnHBcAe Ucpf8YoFnzG40cVnOKg== X-Received: by 2002:a05:620a:c44:b0:508:201b:39d0 with SMTP id u4-20020a05620a0c4400b00508201b39d0mr11605371qki.437.1646778378136; Tue, 08 Mar 2022 14:26:18 -0800 (PST) X-Google-Smtp-Source: ABdhPJz2dWtX1/6u4/b0ihSMhAbzb25fclMvkcfJSp/XOMwz3R3kcowGjUO6DIcfSC51z+y70Admwg== X-Received: by 2002:a05:620a:c44:b0:508:201b:39d0 with SMTP id u4-20020a05620a0c4400b00508201b39d0mr11605355qki.437.1646778377696; Tue, 08 Mar 2022 14:26:17 -0800 (PST) Received: from [192.168.1.149] (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 v129-20020a379387000000b0064936bab2fcsm115829qkd.48.2022.03.08.14.26.16 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 08 Mar 2022 14:26:16 -0800 (PST) Message-ID: Date: Tue, 8 Mar 2022 17:26:15 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [PATCH] c++: detecting copy-init context during CTAD [PR102137] To: Patrick Palka Cc: gcc-patches@gcc.gnu.org References: <20220304182436.122498-1-ppalka@redhat.com> <0c026689-9136-7d66-cc17-fe4fc4c6e5f6@idea> <6962ed32-1424-07c8-c165-0e9441b93433@idea> From: Jason Merrill In-Reply-To: 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.8 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, 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 22:26:24 -0000 On 3/8/22 17:17, Patrick Palka wrote: > 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. Agreed. Not important, just curious. >> >>> 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; >> >> >