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 60A0E3857038 for ; Sat, 17 Sep 2022 14:31:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 60A0E3857038 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=1663425067; 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=oy1gH68P9viakjyyDNGfNl/D6M3hFcIRxkBWnMU7iRw=; b=VLBAwNYStsmYLFqLLLlYvaWIkHwDL2t3Zx9MR+geNa1b5NtBPjndQbtz5kIInocaLzfrVy SzenUetvbgP5yUNTd3Viwt/8TF+w212wb1mMKXpGeklknQl9vOxblpqhnDJlDf3wXSrnIS O4WCCB2dPob13uDKN8aOveiNnf7qhBQ= Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-402-zHfnzgh2MG6v0txpwUXBbg-1; Sat, 17 Sep 2022 10:31:04 -0400 X-MC-Unique: zHfnzgh2MG6v0txpwUXBbg-1 Received: by mail-qv1-f70.google.com with SMTP id c1-20020a0cfb01000000b00495ad218c74so16927859qvp.20 for ; Sat, 17 Sep 2022 07:31:04 -0700 (PDT) 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; bh=oy1gH68P9viakjyyDNGfNl/D6M3hFcIRxkBWnMU7iRw=; b=ayFL/YCCwYcRMZ0LAxsTJbMIe/b6nguEbKvT8xgCII9JzhdLcFeu0E3/kAE1mA1Jp4 KffcX4ZPUNGa6C8GkQ2EJnylqtX0EXz+RMcCAHFmGh5Gh3T+Dk0EKJOGmT3Ap2Sy/1mv InpFvyGvqC5kGeQl56kR4pVTY3lDXZiR5b2ZjfrqDQo7qu1venjxJuosKNG825RWyjeL xvaRZCj1eBs1SgLGgLLebKKqDaffaQASoSFPfRxbXIJNDI8M353T7Hk/vrtKNyq31O9E 5NOtViISTjcH99dVQY3qIGQNLHZQlv7w/+jjI8QlKv6LuHgi6nrFP9kqT4jY+VTvJ04f HlIA== X-Gm-Message-State: ACrzQf0NCIugEUKitaYlexW3nNwltS6Zvl7Y1tIvGcgqRVgHr75w8Otr LsZzUAW4fuaURwwjlmcgUcX2AJV9gUaXEQTMpfA0uSd6iyrK7by/QIUh8R7eIwIvtfzllWjgOQS I8HXz0t29LOYRYxrhAw== X-Received: by 2002:ac8:5fcf:0:b0:35b:d81d:792e with SMTP id k15-20020ac85fcf000000b0035bd81d792emr8579152qta.24.1663425063869; Sat, 17 Sep 2022 07:31:03 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5ujC0w284cKGWayWdbE1cW9S/vz7Oy1VuSdSrNbHQ1RYb/oJ+++YI1jftuDEJDfGGoQkP+Mw== X-Received: by 2002:ac8:5fcf:0:b0:35b:d81d:792e with SMTP id k15-20020ac85fcf000000b0035bd81d792emr8579125qta.24.1663425063507; Sat, 17 Sep 2022 07:31:03 -0700 (PDT) Received: from [192.168.1.130] (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id y20-20020a05620a44d400b006ced5d3f921sm2685334qkp.52.2022.09.17.07.31.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 17 Sep 2022 07:31:02 -0700 (PDT) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Sat, 17 Sep 2022 10:31:02 -0400 (EDT) To: Jason Merrill cc: Patrick Palka , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] c++: constraint matching, TEMPLATE_ID_EXPR, current inst In-Reply-To: Message-ID: References: <20220915155822.4021344-1-ppalka@redhat.com> <549844d1-be60-592b-5e2b-1e4071f6a7f8@redhat.com> <8abd55bf-7f1e-5ab5-895e-a3f42631ec56@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.5 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_LOW,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 Sat, 17 Sep 2022, Jason Merrill wrote: > On 9/16/22 10:59, Patrick Palka wrote: > > On Fri, 16 Sep 2022, Jason Merrill wrote: > > > > > On 9/15/22 11:58, Patrick Palka wrote: > > > > Here we're crashing during constraint matching for the instantiated > > > > hidden friends due to two issues with dependent substitution into a > > > > TEMPLATE_ID_EXPR naming a template from the current instantiation > > > > (as performed from maybe_substitute_reqs_for for C<3> with T=T): > > > > > > > > * tsubst_copy substitutes into such a TEMPLATE_DECL by looking it > > > > up from the substituted class scope. But for this to not fail > > > > when > > > > the args are dependent, we need to pass entering_scope=true for > > > > the > > > > class scope substitution so that we obtain the primary template > > > > type > > > > A (which has TYPE_BINFO) instead of the implicit instantiation > > > > A (which doesn't). > > > > * lookup_and_finish_template_variable shouldn't instantiate a > > > > TEMPLATE_ID_EXPR that names a TEMPLATE_DECL which has more than > > > > one level of (unsubstituted) parameters (such as A::C). > > > > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > > > > trunk? > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > * pt.cc (lookup_and_finish_template_variable): Don't > > > > instantiate if the template's scope is dependent. > > > > (tsubst_copy) : Pass entering_scope=true > > > > when substituting the class scope. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > * g++.dg/cpp2a/concepts-friend10.C: New test. > > > > --- > > > > gcc/cp/pt.cc | 14 +++++++------ > > > > .../g++.dg/cpp2a/concepts-friend10.C | 21 > > > > +++++++++++++++++++ > > > > 2 files changed, 29 insertions(+), 6 deletions(-) > > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend10.C > > > > > > > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > > > > index db4e808adec..bfcbe0b8670 100644 > > > > --- a/gcc/cp/pt.cc > > > > +++ b/gcc/cp/pt.cc > > > > @@ -10475,14 +10475,15 @@ tree > > > > lookup_and_finish_template_variable (tree templ, tree targs, > > > > tsubst_flags_t complain) > > > > { > > > > - templ = lookup_template_variable (templ, targs); > > > > - if (!any_dependent_template_arguments_p (targs)) > > > > + tree var = lookup_template_variable (templ, targs); > > > > + if (TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (templ)) == 1 > > > > + && !any_dependent_template_arguments_p (targs)) > > > > > > I notice that finish_id_expression_1 uses the equivalent of > > > type_dependent_expression_p (var). Does that work here? > > > > Hmm, it does, but kind of by accident: type_dependent_expression_p > > returns true for all variable TEMPLATE_ID_EXPRs because of their empty > > TREE_TYPE (as set by finish_template_variable). So testing t_d_e_p here > > is equivalent to testing processing_template_decl, it seems -- maximally > > conservative. > > > > We can improve type_dependent_expression_p for variable TEMPLATE_ID_EXPR > > by ignoring its (always empty) TREE_TYPE and just considering dependence > > of its template and args directly. > > > > Doing so exposes that value_dependent_expression_p is wrong for > > (non-type-dependent) variable template specializations -- since we don't > > set/track DECL_DEPENDENT_INIT_P for them, > > Hmm, why not? AFAICT we set DECL_DEPENDENT_INIT_P only from cp_finish_decl, which we call when parsing a variable template (or templated static data member or templated local variable IIUC) and when instantiating its definition, but not when specializing it. So if we want to rely on it to determine value dependence of a variable template specialization, it seems we need to also set/propagate DECL_DEPENDENT_INIT_P at specialization time. Note that the flag currently tracks ordinary value dependence of the initializer, but for variable template specializations, it seems we'd want the flag to just track dependence on _outer_ template parameters, because if we're at that point in v_d_e_p, we already know that the innermost arguments are non-dependent (it was ruled out by t_d_e_p), so dependence on the innermost arguments in the initializer shouldn't matter. But this'd mean the flag would have a different meaning depending on the kind of VAR_DECL. To keep things simple, perhaps we should just ignore the flag entirely for variable template specializations (and keep using it for templated static data members and templated local variables)? > > > the VAR_DECL branch ends up > > returning false even if the initializer depends on outer args. Instead, > > I suppose we can give a reasonably conservative answer by considering > > dependence of its enclosing scope as we do for FUNCTION_DECL. > > I wonder why we do that for functions rather than rely on the later > any_dependent_template_arguments_p? Hmm, which call to any_dependent_template_arguments_p do you mean? > Perhaps because checking whether the > scope is dependent is cached, so should be faster. I wonder if it would be > worthwhile to have similar dependent/dependent_valid flags on template arg > vecs.... That sounds handy. > > > Does the following seem reasonable? Bootstrapped and regtested on > > x86_64-pc-linux-gnu. > > > > -- >8 -- > > > > gcc/cp/ChangeLog: > > > > * pt.cc (finish_template_variable): Consider only the innermost > > template parms since we have only the innermost args. > > (lookup_and_finish_template_variable): Check > > type_dependent_expression_p instead. > > (tsubst_copy) : Pass entering_scope=true > > when substituting the class scope. > > (value_dependent_expression_p) : Move below ... > > : ... here. Fall through for variable template > > specializations. > > (type_dependent_expression_p): Handle variable TEMPLATE_ID_EXPR > > precisely. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/cpp1y/noexcept1.C: Expect another ahead of time error. > > * g++.dg/cpp1y/var-templ70.C: New test. > > * g++.dg/cpp2a/concepts-friend10.C: New test. > > --- > > gcc/cp/pt.cc | 53 ++++++++++++------- > > gcc/testsuite/g++.dg/cpp1y/noexcept1.C | 2 +- > > gcc/testsuite/g++.dg/cpp1y/var-templ70.C | 22 ++++++++ > > .../g++.dg/cpp2a/concepts-friend10.C | 24 +++++++++ > > 4 files changed, 82 insertions(+), 19 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ70.C > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend10.C > > > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > > index db4e808adec..88a09891a00 100644 > > --- a/gcc/cp/pt.cc > > +++ b/gcc/cp/pt.cc > > @@ -10447,10 +10447,10 @@ finish_template_variable (tree var, tsubst_flags_t > > complain) > > tree templ = TREE_OPERAND (var, 0); > > tree arglist = TREE_OPERAND (var, 1); > > - tree parms = DECL_TEMPLATE_PARMS (templ); > > - arglist = coerce_innermost_template_parms (parms, arglist, templ, > > complain, > > - /*req_all*/true, > > - /*use_default*/true); > > + tree parms = DECL_INNERMOST_TEMPLATE_PARMS (templ); > > + arglist = coerce_template_parms (parms, arglist, templ, complain, > > + /*req_all*/true, > > + /*use_default*/true); > > if (arglist == error_mark_node) > > return error_mark_node; > > @@ -10475,14 +10475,14 @@ tree > > lookup_and_finish_template_variable (tree templ, tree targs, > > tsubst_flags_t complain) > > { > > - templ = lookup_template_variable (templ, targs); > > - if (!any_dependent_template_arguments_p (targs)) > > + tree var = lookup_template_variable (templ, targs); > > + if (!type_dependent_expression_p (var)) > > { > > - templ = finish_template_variable (templ, complain); > > - mark_used (templ); > > + var = finish_template_variable (var, complain); > > + mark_used (var); > > } > > - return convert_from_reference (templ); > > + return convert_from_reference (var); > > } > > /* If the set of template parameters PARMS contains a template parameter > > @@ -17282,7 +17282,8 @@ tsubst_copy (tree t, tree args, tsubst_flags_t > > complain, tree in_decl) > > TEMPLATE_DECL with `D' as its DECL_CONTEXT. Now we > > have to substitute this with one having context `D'. */ > > - tree context = tsubst (DECL_CONTEXT (t), args, complain, in_decl); > > + tree context = tsubst_aggr_type (DECL_CONTEXT (t), args, complain, > > + in_decl, /*entering_scope=*/true); > > return lookup_field (context, DECL_NAME(t), 0, false); > > } > > else > > @@ -27684,13 +27685,6 @@ value_dependent_expression_p (tree expression) > > /* A dependent member function of the current instantiation. */ > > return dependent_type_p (BINFO_TYPE (BASELINK_BINFO (expression))); > > - case FUNCTION_DECL: > > - /* A dependent member function of the current instantiation. */ > > - if (DECL_CLASS_SCOPE_P (expression) > > - && dependent_type_p (DECL_CONTEXT (expression))) > > - return true; > > - break; > > - > > case IDENTIFIER_NODE: > > /* A name that has not been looked up -- must be dependent. */ > > return true; > > @@ -27725,7 +27719,19 @@ value_dependent_expression_p (tree expression) > > && value_expr == error_mark_node)) > > return true; > > } > > - return false; > > + if (!variable_template_specialization_p (expression)) > > + /* For variable template specializations, also consider dependence > > + of the enclosing scope. For other specializations it seems we > > + can trust DECL_DEPENDENT_INIT_P. */ > > + return false; > > + /* Fall through. */ > > + > > + case FUNCTION_DECL: > > + /* A dependent member of the current instantiation. */ > > + if (DECL_CLASS_SCOPE_P (expression) > > + && dependent_type_p (DECL_CONTEXT (expression))) > > + return true; > > + break; > > case DYNAMIC_CAST_EXPR: > > case STATIC_CAST_EXPR: > > @@ -28095,6 +28101,17 @@ type_dependent_expression_p (tree expression) > > expression = BASELINK_FUNCTIONS (expression); > > } > > + /* A variable TEMPLATE_ID_EXPR is type-dependent iff the template or > > + arguments are. */ > > + if (TREE_CODE (expression) == TEMPLATE_ID_EXPR) > > + { > > + tree tmpl = TREE_OPERAND (expression, 0); > > + tree args = TREE_OPERAND (expression, 1); > > + if (variable_template_p (tmpl) && !variable_concept_p (tmpl)) > > + return type_dependent_expression_p (tmpl) > > + || any_dependent_template_arguments_p (args); > > + } > > + > > /* A function or variable template-id is type-dependent if it has any > > dependent template arguments. */ > > if (VAR_OR_FUNCTION_DECL_P (expression) > > diff --git a/gcc/testsuite/g++.dg/cpp1y/noexcept1.C > > b/gcc/testsuite/g++.dg/cpp1y/noexcept1.C > > index 86e46c96148..a660b9d6c9e 100644 > > --- a/gcc/testsuite/g++.dg/cpp1y/noexcept1.C > > +++ b/gcc/testsuite/g++.dg/cpp1y/noexcept1.C > > @@ -8,6 +8,6 @@ struct C { > > template friend int foo() noexcept(b<1>); // { dg-error "not > > usable in a constant expression|different exception specifier" } > > }; > > -template int foo() noexcept(b<1>); > > +template int foo() noexcept(b<1>); // { dg-error "not usable in > > a constant expression" } > > auto a = C(); > > diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ70.C > > b/gcc/testsuite/g++.dg/cpp1y/var-templ70.C > > new file mode 100644 > > index 00000000000..e1040165c34 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp1y/var-templ70.C > > @@ -0,0 +1,22 @@ > > +// Verify we correctly compute value dependence for variable template > > +// specializations. > > +// { dg-do compile { target c++17 } } > > + > > +template static constexpr int value = false; > > + > > +template > > +void f() { > > + // value is not dependent, so we can check this ahead of time. > > + static_assert(value, ""); // { dg-error "assertion failed" } > > +} > > + > > +template > > +struct A { > > + template static constexpr bool member = T(); > > + auto f() { > > + // member is not type dependent, but we consider it value > > dependent > > + // since it's a member of the current instantiation, so we don't check > > + // this ahead of time. > > + static_assert(member, ""); > > + } > > +}; > > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend10.C > > b/gcc/testsuite/g++.dg/cpp2a/concepts-friend10.C > > new file mode 100644 > > index 00000000000..bb5e1e6038b > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend10.C > > @@ -0,0 +1,24 @@ > > +// Verify we don't crash during constraint matching containing a > > +// TEMPLATE_ID_EXPR that names a template from the current instantiation. > > +// { dg-do compile { target c++20 } } > > + > > +template static constexpr bool False = false; > > + > > +template > > +struct A { > > + template static constexpr bool C = sizeof(T) > N; > > + friend constexpr void f(A) requires C<1> { } > > + friend constexpr void f(A) requires C<1> && False { } > > +}; > > + > > +template > > +struct A { > > + template static constexpr bool C = sizeof(T) > N; > > + friend constexpr void g(A) requires C<1> { } > > + friend constexpr void g(A) requires C<1> && False { } > > +}; > > + > > +int main() { > > + f(A{}); > > + g(A{}); > > +} > >