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.129.124]) by sourceware.org (Postfix) with ESMTPS id 272373858D35 for ; Tue, 8 Mar 2022 20:07:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 272373858D35 Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-351-ViYpFb9LPkeQRN_lWowZ2w-1; Tue, 08 Mar 2022 15:07:35 -0500 X-MC-Unique: ViYpFb9LPkeQRN_lWowZ2w-1 Received: by mail-qt1-f199.google.com with SMTP id f22-20020ac840d6000000b002dd4d87de21so38424qtm.23 for ; Tue, 08 Mar 2022 12:07:35 -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=W+yujuiBVOaGqq+7W6A4NVty5Jk+I8zoPFcR9aAmEF8=; b=F/rK/VkuHuXDPDYRV7RUfkyCZpCjUS6tgWto1Bf+azGo0WP5B645zpMAE0xcEMMLPE vMahuH5HENhHIcCUXZB7Recxyktv/UaHiM0SA6vcuLOwzjWfx/JxWE0+BipYV90/6S84 V3S5EDhivndA+kH3GJa9PGv6AJlC9QxI+OEousAlWqsAawseZPr8b1JChtJa5UpZFnCC XuQTyreasjSr1uIkDKXc+yk+Dbx/UKWh4uJ7uG6wK0dF1zUY/P/XlShyOtF1EOa+JJDN AkO+DY0xgM9Xs+ewsnhXtKeY9sZMk+0QGbujS3TDw1syxhiSsSV7Bq7ZdJssv8DqOokf ZONA== X-Gm-Message-State: AOAM532RlDC4G/UIhDhvGv+8/43w8bartPwWC0WyBD5hBTEdCFPuxI9c 9jxz9VGYbGWY2cZL6U1LwQrxqP4jNAfftxog+aNjHJpRpPg6KsbW/c0PdPDUesAFi7fNGXy+7M9 51sT9vxWn9SZUEz7yYg== X-Received: by 2002:ac8:5b81:0:b0:2df:fe81:7c8b with SMTP id a1-20020ac85b81000000b002dffe817c8bmr15320118qta.228.1646770054904; Tue, 08 Mar 2022 12:07:34 -0800 (PST) X-Google-Smtp-Source: ABdhPJyfti7BzuhWcCqbMpEUo2lj6JsyBaNf9GIquQo8TCwq3QAn+cXHp/84X2+uPfe79s1Hu0DOlA== X-Received: by 2002:ac8:5b81:0:b0:2df:fe81:7c8b with SMTP id a1-20020ac85b81000000b002dffe817c8bmr15320085qta.228.1646770054420; Tue, 08 Mar 2022 12:07:34 -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 d67-20020a37b446000000b0067b336031c6sm2623093qkf.31.2022.03.08.12.07.33 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 08 Mar 2022 12:07:33 -0800 (PST) Message-ID: Date: Tue, 8 Mar 2022 15:07:31 -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: <6962ed32-1424-07c8-c165-0e9441b93433@idea> 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 20:07:41 -0000 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. > 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;