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 D5AEC3858C52 for ; Fri, 1 Apr 2022 18:06:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D5AEC3858C52 Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-400-JN4yBV1rOjO_IGl_ufBWaQ-1; Fri, 01 Apr 2022 14:06:10 -0400 X-MC-Unique: JN4yBV1rOjO_IGl_ufBWaQ-1 Received: by mail-qv1-f69.google.com with SMTP id cl19-20020a05621404f300b0044103143211so2595616qvb.10 for ; Fri, 01 Apr 2022 11:06:10 -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=Xn18yM2vRCe2IXKLLNacKpBGcjL62XI7tibQQsbxVzo=; b=WtXYQ5G+EtOmI83LX8NQCRzn7j1nrP+qmB+XA5sgn+tqZM8bFgQH37MYb8ErhKNmea ca0Kl3i25K+ypRTpLkDtJcLbAKBwYAAdrZKMKpy+VjhuPqUqyXFS6bnZfjLcmHVHM95c +1QRrYTdRSS0E8hVcv3cCP4+644wOuL2brfMB5qAe4k7Df1UASn5KstayoIqtLo6nIRf ZHFFpHn0UB1D0uEr+4eJJmDiva0n2S5WxM0oLqVXcfXvKvM8DftB95msohV5iUTuSiWd mKWcVXMbLTDQseqkcfNcBcHiWrWZg7q2/X/D1HsvhPzp1v+UEl0eC2SaCgTaKq1hXxp+ PVCw== X-Gm-Message-State: AOAM531qD0YOIuJsNTZrPCPeOloTshP3taAEF/2LIMSzvMYmODPdyU2c bOn0X0+vcFV8eQ1lZcJjyTW/cgtqlOi0ssOTcsH2n5E9yOcLoIYmifK0/nwpFMKGjKoKri5lnrl g8tsdSkX2ad3y6Z58Cw== X-Received: by 2002:ae9:e00a:0:b0:699:a3e5:39ee with SMTP id m10-20020ae9e00a000000b00699a3e539eemr1950803qkk.39.1648836368980; Fri, 01 Apr 2022 11:06:08 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxv5lvfqZ8tH9fTO1nA4RIDk4U2tD1itD7PS4UJ9wNatIMfYsB6lmrMOqAmYP9grdG6Ht9m7w== X-Received: by 2002:ae9:e00a:0:b0:699:a3e5:39ee with SMTP id m10-20020ae9e00a000000b00699a3e539eemr1950777qkk.39.1648836368458; Fri, 01 Apr 2022 11:06:08 -0700 (PDT) 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 s13-20020a05620a0bcd00b0067afe7dd3ffsm1818440qki.49.2022.04.01.11.06.07 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 01 Apr 2022 11:06:07 -0700 (PDT) Message-ID: <74840bc2-b868-befb-0ec3-810cf57281db@redhat.com> Date: Fri, 1 Apr 2022 14:06:05 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Subject: Re: [PATCH] c++: implicit guides should inherit class constraints [PR104873] To: Patrick Palka , gcc-patches@gcc.gnu.org References: <20220401151741.2182149-1-ppalka@redhat.com> From: Jason Merrill In-Reply-To: <20220401151741.2182149-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.9 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_LOW, 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: Fri, 01 Apr 2022 18:06:14 -0000 On 4/1/22 11:17, Patrick Palka wrote: > An implicit guide already inherits the (rewritten) constraints of the > constructor. Thus it seems natural that the guide must also inherit > the constraints of the class template, since a constructor's constraints > might assume the class's constraints are satisfied, and therefore > checking these two sets of constraints "out of order" may result in hard > errors as in the first testcase below. > This patch makes implicit guides inherit the constraints of the class > template (even for unconstrained constructors, and even for the copy > deduction candidate). > > In passing, this patch gives implicit guides a trailing return type > since that's how they're depicted in the standard (e.g. > [over.match.class.deduct]/6); this changes the order of substitution > into implicit guides in a probably negligible way, especially now that > they inherit the class constraints. > > The parameter_mapping_equivalent_p change is to avoid an ICE in the last > testcase below (described within), reduced from a cmcstl2 testsuite ICE. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look like > the right approach? I don't think so, given the testcases below. Maybe fn_type_unification should check formation of the return type of a deduction guide before constraints? In general, whichever order you do things in, it'll be wrong for some testcase or other. The broader subject of constraints and deduction guides should be raised with CWG in general (https://github.com/cplusplus/CWG/issues/new/choose) > PR c++/104873 > > gcc/cp/ChangeLog: > > * constraint.cc (parameter_mapping_equivalent_p): Relax assert > to expect equivalence not identity of template parameters. > * pt.cc (build_deduction_guide): Propagate the class's > constraints to the deduction guide. Set TYPE_HAS_LATE_RETURN_TYPE > on the function type. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp2a/concepts-ctad5.C: New test. > * g++.dg/cpp2a/concepts-ctad6.C: New test. > * g++.dg/cpp2a/concepts-ctad6a.C: New test. > * g++.dg/cpp2a/concepts-ctad7.C: New test. > --- > gcc/cp/constraint.cc | 2 +- > gcc/cp/pt.cc | 26 ++++++++++++++++++ > gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C | 29 ++++++++++++++++++++ > gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C | 19 +++++++++++++ > gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C | 19 +++++++++++++ > gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C | 26 ++++++++++++++++++ > 6 files changed, 120 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > index 94f6222b436..6cbb182dda2 100644 > --- a/gcc/cp/constraint.cc > +++ b/gcc/cp/constraint.cc > @@ -604,7 +604,7 @@ parameter_mapping_equivalent_p (tree t1, tree t2) > tree map2 = ATOMIC_CONSTR_MAP (t2); > while (map1 && map2) > { > - gcc_checking_assert (TREE_VALUE (map1) == TREE_VALUE (map2)); > + gcc_checking_assert (cp_tree_equal (TREE_VALUE (map1), TREE_VALUE (map2))); > tree arg1 = TREE_PURPOSE (map1); > tree arg2 = TREE_PURPOSE (map2); > if (!template_args_equal (arg1, arg2)) > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index 75ed9a34018..966e6d90d3a 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -29261,6 +29261,10 @@ build_deduction_guide (tree type, tree ctor, tree outer_args, tsubst_flags_t com > /* Discard the 'this' parameter. */ > fparms = FUNCTION_ARG_CHAIN (ctor); > fargs = TREE_CHAIN (DECL_ARGUMENTS (ctor)); > + /* The guide's constraints consist of the class template's constraints > + followed by the constructor's rewritten constraints. We start > + with the constructor's constraints (since we need to rewrite them), > + and prepend the class template's constraints later. */ > ci = get_constraints (ctor); > loc = DECL_SOURCE_LOCATION (ctor); > explicit_p = DECL_NONCONVERTING_P (ctor); > @@ -29362,6 +29366,27 @@ build_deduction_guide (tree type, tree ctor, tree outer_args, tsubst_flags_t com > return error_mark_node; > } > > + /* Prepend the class template's constraints to the constructor's rewritten > + constraints (if any). */ > + if (tree class_ci = get_constraints (CLASSTYPE_TI_TEMPLATE (type))) > + { > + if (outer_args) > + { > + /* FIXME: As above. */ > + ++processing_template_decl; > + class_ci = tsubst_constraint_info (class_ci, outer_args, > + complain, ctor); > + --processing_template_decl; > + } > + if (ci) > + ci = build_constraints (combine_constraint_expressions > + (CI_TEMPLATE_REQS (class_ci), > + CI_TEMPLATE_REQS (ci)), > + CI_DECLARATOR_REQS (ci)); > + else > + ci = copy_node (class_ci); > + } > + > if (!memtmpl) > { > /* Copy the parms so we can set DECL_PRIMARY_TEMPLATE. */ > @@ -29371,6 +29396,7 @@ build_deduction_guide (tree type, tree ctor, tree outer_args, tsubst_flags_t com > } > > tree fntype = build_function_type (type, fparms); > + TYPE_HAS_LATE_RETURN_TYPE (fntype) = true; > tree ded_fn = build_lang_decl_loc (loc, > FUNCTION_DECL, > dguide_name (type), fntype); > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C > new file mode 100644 > index 00000000000..5990088f1db > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C > @@ -0,0 +1,29 @@ > +// PR c++/104873 > +// { dg-do compile { target c++20 } } > +// Verify implicit guides inherit the constraints of the class template. > + > +template > +struct A { > + static_assert(!__is_same(T, void)); > + static constexpr bool value = true; > +}; > + > +template requires (!__is_same(T, void)) > +struct B { > + B(T*) requires A::value; // #1 > + // implicit guide: > + // template requires (!__is_same(T, void)) && A::value > + // B(T*) -> B; > + > + B(T); > + // implicit guide: > + // template requires (!__is_same(T, void)) > + // B(T) -> B; > +}; > + > +void* p; > +using type = decltype(B(p)); // previously hard error during dguide overload > + // resolution because #1's implicit guide would > + // inherit only the constructor's constraints and > + // not also the class's. > +using type = B; > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C > new file mode 100644 > index 00000000000..10bb86df6a0 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C > @@ -0,0 +1,19 @@ > +// PR c++/104873 > +// { dg-do compile { target c++20 } } > + > +template concept C = true; > + > +template requires C > +struct S { > + S(...); > +}; > +// synthesized copy deduction candidate: > +// template requires C > +// S(S) -> S; > + > +template S(S) -> S; // #1 > + > +using type = decltype(S(S())); // The deduction candidate (which inherits > + // the class's constraints) is preferred > + // over #1 because it's more constrained. This seems like a regression: Presumably people write deduction guides because they want them to be used, not to have them silently ignored because the class is constrained. > +using type = S; > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C > new file mode 100644 > index 00000000000..f9d1d6ec11e > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C > @@ -0,0 +1,19 @@ > +// PR c++/104873 > +// { dg-do compile { target c++20 } } > + > +template concept C = true; > + > +template requires C > +struct S { > + S(T); // #1 > + // implicit guide: > + // template requires C > + // S -> S; > +}; > + > +template S(T) -> S>; // #2 > + > +using type = decltype(S(0)); // The implicit guide for #1 (which inherits the > + // class's constraints) is preferred over #2 > + // because it's more constrained. Likewise. > +using type = S; > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C > new file mode 100644 > index 00000000000..4e1b1abfb94 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C > @@ -0,0 +1,26 @@ > +// { dg-do compile { target c++20 } } > + > +template requires __is_same(T, int) > +struct A; > + > +// When checking constraints on the class template, we > +// normalize __is_same(T, int) and cache the resulting atom. > +A *p; > + > +template requires __is_same(T, int) > +struct A { > + A(T); > + // implicit guide: > + // template requires __is_same(T, int) > + // A(T) -> A; > +}; > + > +// When checking constraints on the implicit guide, we normalize > +// the same expression again with an equivalent but not identical > +// set of template parameters (those from the definition vs from > +// the forward declaration). We should notice that that the > +// resulting atom is the same as the one we cached earlier, but > +// instead we crashed due to an overly strict assert in > +// parameter_mapping_equivalent_p that demanded identity instead > +// of equivalence of the template parameters in the mapping. > +A a = 1;