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 53AF53858D3C for ; Mon, 19 Dec 2022 16:01:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 53AF53858D3C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1671465678; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Ph58DoVZNQcjpMVgfJNphatlrOCXUk9NfjeAfQ/Mmh8=; b=fQdw9gsd6a5lblRwl1mb6uP6JiPAphmoNMGQ7hGyMI29QXGmzP3Z/OqSBSc3JlZaTcebAG wXN1oWAKOsebldOm9EOywKsaBLWWKMH7bSpHJHcn8VVIgyTOPon40leEm21DnNd9pWPlAA +ovOXseaoMAKJ0izgt+V1+59PWgMgqI= Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-159-kMSx6pxQO_y3BDxwpXcAsA-1; Mon, 19 Dec 2022 11:01:14 -0500 X-MC-Unique: kMSx6pxQO_y3BDxwpXcAsA-1 Received: by mail-qt1-f198.google.com with SMTP id bt4-20020ac86904000000b003a96b35e7a8so4135102qtb.8 for ; Mon, 19 Dec 2022 08:01:14 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:references:message-id:in-reply-to:subject:cc:to:date :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Ph58DoVZNQcjpMVgfJNphatlrOCXUk9NfjeAfQ/Mmh8=; b=t0CGNQC+KqQG6XMo6AFrqnR8i5+oHNIa9bfKI0CFzwU87dC6bBfz4rdzTF44HbU42d d43PyPsQZYwKlDWgfYTlPHX1IqwEBtTSYePTwaJiN1705bUIKtHEcp/NN25Un90jCCmg 0X7MkZZw2r1sJhA39UoFdqZWMOKfufcCAxJyyUvBgeHUoF0kn+ykI+rJl4h1uf4evoVa zTm7j5uutj7yJiYNCrCW6qWZvReRGNb60iQ+DC2ZAhIWYVC1ocBp+hipQOwLwUEfFynb zx9NEwzqCxnkWBapuhJ7809I/kJTwxqZVn7sklhwoiy42DbXjlhHL8yOB28NuuMwHlw/ 4Wag== X-Gm-Message-State: ANoB5pkjNq3xGlKNAsUDQ4QfWQOrvRw9qKxz/ykQTRCTHD4WuJZbY/Wf d8WRPTwy0L67asJk0pDf8joFIZTkKpnyRzOcV7UU8DjvKFQ/FDaOTZXO60xk/cRc/us3BIOlbNG WdoEEi5nE/aAPb/xnZQ== X-Received: by 2002:a05:622a:1a26:b0:39c:da21:6c01 with SMTP id f38-20020a05622a1a2600b0039cda216c01mr77115874qtb.3.1671465673043; Mon, 19 Dec 2022 08:01:13 -0800 (PST) X-Google-Smtp-Source: AA0mqf712JRatPzwvYzRkbvO2fQsWo7y42cFck8y+D55b8kkAd94TjIKA8yhzbFkeXV/ap5kJSmmKA== X-Received: by 2002:a05:622a:1a26:b0:39c:da21:6c01 with SMTP id f38-20020a05622a1a2600b0039cda216c01mr77115811qtb.3.1671465672612; Mon, 19 Dec 2022 08:01:12 -0800 (PST) Received: from [192.168.1.130] (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id w7-20020ac86b07000000b003a50d92f9b4sm6190776qts.1.2022.12.19.08.01.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Dec 2022 08:01:12 -0800 (PST) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Mon, 19 Dec 2022 11:01:11 -0500 (EST) To: Patrick Palka cc: gcc-patches@gcc.gnu.org, jason@redhat.com Subject: Re: [PATCH] c++: NTTP object wrapper substitution fixes [PR103346, ...] In-Reply-To: <98f22062-9f4e-0406-cddc-cfd49a63c3e8@idea> Message-ID: <8903c224-1b5d-0bf1-4319-862df8f6e414@idea> References: <20221206175702.987794-1-ppalka@redhat.com> <98f22062-9f4e-0406-cddc-cfd49a63c3e8@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=-13.9 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_H2,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 List-Id: On Tue, 6 Dec 2022, Patrick Palka wrote: > On Tue, 6 Dec 2022, Patrick Palka wrote: > > > This patch fixes some issues with substitution into a C++20 template > > parameter object wrapper: > > > > * The first testcase demonstrates a situation where the same_type_p > > assert in relevant case of tsubst_copy doesn't hold, because (partial) > > substitution of {int,} into the VIEW_CONVERT_EXPR wrapper yields > > A but substitution into the underlying TEMPLATE_PARM_INDEX is a > > nop and yields A due to tsubst's level == 1 early exit test. So > > this patch just gets rid of the assert; the type mismatch doesn't > > seem to be a problem in practice since the coercion is from one > > dependent type to another. > > > > * In the second testcase, dependent substitution into the underlying > > TEMPLATE_PARM_INDEX yields a CALL_EXPR with empty TREE_TYPE, which > > tsubst_copy doesn't expect. This patch fixes this by handling empty > > TREE_TYPE the same way as a non-const TREE_TYPE. Moreover, after > > this substitution we're left with a VIEW_CONVERT_EXPR wrapping a > > CALL_EXPR instead of a TEMPLATE_PARM_INDEX, which during the subsequent > > non-dependent substitution tsubst_copy doesn't expect either. So > > this patch also relaxes the tsubst_copy case to accept such > > VIEW_CONVERT_EXPR too. > > > > * In the third testcase, we end up never resolving the call to > > f.modify() since tsubst_copy doesn't do overload resolution. > > This patch fixes this by moving the handling of these > > VIEW_CONVERT_EXPR wrappers from tsubst_copy to tsubst_copy_and_build. > > And it turns out (at least according to our testsuite) that > > tsubst_copy doesn't directly need to handle the other kinds of > > NON_LVALUE_EXPR and VIEW_CONVERT_EXPR, so this patch also gets rid > > of the location_wrapper_p handling from tsubst_copy and moves the > > REF_PARENTHESIZED_P handling to tsubst_copy_and_build. > > On second thought, getting rid of the location_wrapper_p and > REF_PARENTHESIZED_P handling from tsubst_copy is perhaps too > risky at this stage. The following patch instead just moves > the tparm object wrapper handling from tsubst_copy to > tsubst_copy_and_build and leaves the rest of tsubst_copy alone. > Smoke tested so far, full bootstrap+regtest in progress: > > -- >8-- > > Subject: [PATCH] c++: NTTP object wrapper substitution fixes [PR103346, ...] > > This patch fixes some issues with substitution into a C++20 template > parameter object wrapper: > > * The first testcase demonstrates a situation where the same_type_p > assert in relevant case of tsubst_copy doesn't hold, because (partial) > substitution of {int,} into the VIEW_CONVERT_EXPR wrapper yields > A but substitution into the underlying TEMPLATE_PARM_INDEX is a > nop and yields A due to tsubst's level == 1 early exit test. So > this patch just gets rid of the assert; the type mismatch doesn't > seem to be a problem in practice, I suppose because the coercion is > from one dependent type to another. > > * In the second testcase, dependent substitution into the underlying > TEMPLATE_PARM_INDEX yields a CALL_EXPR with empty TREE_TYPE, which > tsubst_copy doesn't expect. This patch fixes this by handling empty > TREE_TYPE the same way as a non-const TREE_TYPE. Moreover, after > this substitution we're left with a VIEW_CONVERT_EXPR wrapping a > CALL_EXPR instead of a TEMPLATE_PARM_INDEX, which during the subsequent > non-dependent substitution tsubst_copy doesn't expect either. So > this patch also relaxes tsubst_copy to accept such VIEW_CONVERT_EXPR > too. > > * In the third testcase, we end up never resolving the call to > f.modify() since tsubst_copy doesn't do overload resolution. > This patch fixes this by moving the handling of these > VIEW_CONVERT_EXPR wrappers from tsubst_copy to tsubst_copy_and_build. > For good measure tsubst_copy_and_build should also handle > REF_PARENTHESIZED_P wrappers instead of delegating to tsubst_copy. > > After this patch, VIEW_CONVERT_EXPR substitution is ultimately just > moved from tsubst_copy to tsubst_copy_and_build and made more > permissive. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk? Ping. > > PR c++/103346 > PR c++/104278 > PR c++/102553 > > gcc/cp/ChangeLog: > > * pt.cc (tsubst_copy) : In the handling > of C++20 template parameter object wrappers: Remove same_type_p > assert. Accept non-TEMPLATE_PARM_INDEX inner operand. Handle > empty TREE_TYPE on substituted inner operand. Move it to ... > (tsubst_copy_and_build): ... here. Also handle REF_PARENTHESIZED_P > VIEW_CONVERT_EXPRs. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp2a/nontype-class52a.C: New test. > * g++.dg/cpp2a/nontype-class53.C: New test. > * g++.dg/cpp2a/nontype-class54.C: New test. > * g++.dg/cpp2a/nontype-class55.C: New test. > --- > gcc/cp/pt.cc | 73 ++++++++++--------- > gcc/testsuite/g++.dg/cpp2a/nontype-class52a.C | 15 ++++ > gcc/testsuite/g++.dg/cpp2a/nontype-class53.C | 25 +++++++ > gcc/testsuite/g++.dg/cpp2a/nontype-class54.C | 23 ++++++ > gcc/testsuite/g++.dg/cpp2a/nontype-class55.C | 15 ++++ > 5 files changed, 116 insertions(+), 35 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class52a.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class53.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class54.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class55.C > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index 2d8e4fdd4b5..0a196f069ad 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -17271,42 +17271,16 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl) > return maybe_wrap_with_location (op0, EXPR_LOCATION (t)); > } > tree op = TREE_OPERAND (t, 0); > - if (code == VIEW_CONVERT_EXPR > - && TREE_CODE (op) == TEMPLATE_PARM_INDEX) > - { > - /* Wrapper to make a C++20 template parameter object const. */ > - op = tsubst_copy (op, args, complain, in_decl); > - if (!CP_TYPE_CONST_P (TREE_TYPE (op))) > - { > - /* The template argument is not const, presumably because > - it is still dependent, and so not the const template parm > - object. */ > - tree type = tsubst (TREE_TYPE (t), args, complain, in_decl); > - gcc_checking_assert (same_type_ignoring_top_level_qualifiers_p > - (type, TREE_TYPE (op))); > - if (TREE_CODE (op) == CONSTRUCTOR > - || TREE_CODE (op) == IMPLICIT_CONV_EXPR) > - { > - /* Don't add a wrapper to these. */ > - op = copy_node (op); > - TREE_TYPE (op) = type; > - } > - else > - /* Do add a wrapper otherwise (in particular, if op is > - another TEMPLATE_PARM_INDEX). */ > - op = build1 (code, type, op); > - } > - return op; > - } > /* force_paren_expr can also create a VIEW_CONVERT_EXPR. */ > - else if (code == VIEW_CONVERT_EXPR && REF_PARENTHESIZED_P (t)) > + if (code == VIEW_CONVERT_EXPR && REF_PARENTHESIZED_P (t)) > { > op = tsubst_copy (op, args, complain, in_decl); > op = build1 (code, TREE_TYPE (op), op); > REF_PARENTHESIZED_P (op) = true; > return op; > } > - /* We shouldn't see any other uses of these in templates. */ > + /* We shouldn't see any other uses of these in templates > + (tsubst_copy_and_build handles C++20 tparm object wrappers). */ > gcc_unreachable (); > } > > @@ -21569,12 +21543,41 @@ tsubst_copy_and_build (tree t, > > case NON_LVALUE_EXPR: > case VIEW_CONVERT_EXPR: > - if (location_wrapper_p (t)) > - /* We need to do this here as well as in tsubst_copy so we get the > - other tsubst_copy_and_build semantics for a PARM_DECL operand. */ > - RETURN (maybe_wrap_with_location (RECUR (TREE_OPERAND (t, 0)), > - EXPR_LOCATION (t))); > - /* fallthrough. */ > + { > + tree op = RECUR (TREE_OPERAND (t, 0)); > + if (location_wrapper_p (t)) > + /* We need to do this here as well as in tsubst_copy so we get the > + other tsubst_copy_and_build semantics for a PARM_DECL operand. */ > + RETURN (maybe_wrap_with_location (op, EXPR_LOCATION (t))); > + > + gcc_checking_assert (TREE_CODE (t) == VIEW_CONVERT_EXPR); > + if (REF_PARENTHESIZED_P (t)) > + /* force_paren_expr can also create a VIEW_CONVERT_EXPR. */ > + RETURN (finish_parenthesized_expr (op)); > + > + /* Otherwise, we're dealing with a wrapper to make a C++20 template > + parameter object const. */ > + if (TREE_TYPE (op) == NULL_TREE > + || !CP_TYPE_CONST_P (TREE_TYPE (op))) > + { > + /* The template argument is not const, presumably because > + it is still dependent, and so not the const template parm > + object. */ > + tree type = tsubst (TREE_TYPE (t), args, complain, in_decl); > + if (TREE_CODE (op) == CONSTRUCTOR > + || TREE_CODE (op) == IMPLICIT_CONV_EXPR) > + { > + /* Don't add a wrapper to these. */ > + op = copy_node (op); > + TREE_TYPE (op) = type; > + } > + else > + /* Do add a wrapper otherwise (in particular, if op is > + another TEMPLATE_PARM_INDEX). */ > + op = build1 (VIEW_CONVERT_EXPR, type, op); > + } > + RETURN (op); > + } > > default: > /* Handle Objective-C++ constructs, if appropriate. */ > diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class52a.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class52a.C > new file mode 100644 > index 00000000000..ae5d5df70ac > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class52a.C > @@ -0,0 +1,15 @@ > +// A version of nontype-class52.C where explicit template arguments are > +// given in the call to f (which during deduction need to be partially > +// substituted into the NTTP object V in f's signature). > +// { dg-do compile { target c++20 } } > + > +template struct A { }; > + > +template struct B { }; > + > +template V> void f(B); > + > +int main() { > + constexpr A a; > + f(B{}); > +} > diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class53.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class53.C > new file mode 100644 > index 00000000000..9a6398c5f57 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class53.C > @@ -0,0 +1,25 @@ > +// PR c++/103346 > +// { dg-do compile { target c++20 } } > + > +struct Item {}; > + > +template > +struct Sequence { }; > + > +template > +using ItemSequence = Sequence; > + > +template > +constexpr auto f() { > + constexpr auto l = [](Item item) { return item; }; > + return ItemSequence{}; > +} > + > +using ty0 = decltype(f<>()); > +using ty0 = ItemSequence<>; > + > +using ty1 = decltype(f<{}>()); > +using ty1 = ItemSequence<{}>; > + > +using ty3 = decltype(f<{}, {}, {}>()); > +using ty3 = ItemSequence<{}, {}, {}>; > diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class54.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class54.C > new file mode 100644 > index 00000000000..8127b1f5426 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class54.C > @@ -0,0 +1,23 @@ > +// PR c++/104278 > +// { dg-do compile { target c++20 } } > + > +struct foo { > + int value; > + constexpr foo modify() const { return { value + 1 }; } > +}; > + > +template > +struct bar { > + static void run() { } > +}; > + > +template > +struct qux { > + static void run() { > + bar::run(); > + } > +}; > + > +int main() { > + qux::run(); > +} > diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class55.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class55.C > new file mode 100644 > index 00000000000..afcb3d64495 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class55.C > @@ -0,0 +1,15 @@ > +// PR c++/102553 > +// { dg-do compile { target c++20 } } > + > +struct s1{}; > +template inline constexpr s1 ch{}; > + > +template struct s2{}; > +template using alias1 = s2; > + > +template > +void general(int n) { > + alias1>{}; > +} > + > +template void general(int); > -- > 2.39.0.rc2 > >