On Fri, 16 Sep 2022, 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<T> (which has TYPE_BINFO) instead of the implicit instantiation
> > > A<T> (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<T>::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) <case TEMPLATE_DECL>: 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).
... as set by lookup_template_variable, rather
> 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, 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.
>
> 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) <case TEMPLATE_DECL>: Pass entering_scope=true
> when substituting the class scope.
> (value_dependent_expression_p) <case FUNCTION_DECL>: Move below ...
> <case VAR_DECL>: ... 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<T>' as its DECL_CONTEXT. Now we
> have to substitute this with one having context `D<int>'. */
>
> - 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 <typename> friend int foo() noexcept(b<1>); // { dg-error "not
> usable in a constant expression|different exception specifier" }
> };
>
> -template <typename> int foo() noexcept(b<1>);
> +template <typename> int foo() noexcept(b<1>); // { dg-error "not usable in a
> constant expression" }
>
> auto a = C<int>();
> 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<class T> static constexpr int value = false;
> +
> +template<class T>
> +void f() {
> + // value<int> is not dependent, so we can check this ahead of time.
> + static_assert(value<int>, ""); // { dg-error "assertion failed" }
> +}
> +
> +template<class T>
> +struct A {
> + template<class U> static constexpr bool member = T();
> + auto f() {
> + // member<int> 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<int>, "");
> + }
> +};
> 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<class T> static constexpr bool False = false;
> +
> +template<class T>
> +struct A {
> + template<int N> static constexpr bool C = sizeof(T) > N;
> + friend constexpr void f(A) requires C<1> { }
> + friend constexpr void f(A) requires C<1> && False<T> { }
> +};
> +
> +template<class T>
> +struct A<T*> {
> + template<int N> static constexpr bool C = sizeof(T) > N;
> + friend constexpr void g(A) requires C<1> { }
> + friend constexpr void g(A) requires C<1> && False<T> { }
> +};
> +
> +int main() {
> + f(A<int>{});
> + g(A<int*>{});
> +}
> --
> 2.38.0.rc0
>
>
