c++: constraint matching, TEMPLATE_ID_EXPR, current inst

  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
  

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?

>       {
> -      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 +17283,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

This hunk is OK.

> 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..4b21a379f59
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend10.C
> @@ -0,0 +1,21 @@
> +// { dg-do compile { target c++20 } }
> +// Verify we don't crash during constraint matching containing
> +// a TEMPLATE_ID_EXPR referring to a template from the current
> +// instantiation.
> +
> +template<class T>
> +struct A {
> +  template<int N> static constexpr bool C = sizeof(T) > N;
> +  friend constexpr void f(A) requires C<3> { }
> +  friend constexpr void f(A) requires C<3> || true { }
> +};
> +
> +template<class T>
> +struct A<T*> {
> +  template<int N> static constexpr bool C = sizeof(T) > N;
> +  friend constexpr void g(A) requires C<3> { }
> +  friend constexpr void g(A) requires C<3> || true { }
> +};
> +
> +template struct A<int>;
> +template struct A<int*>;
  

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).  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*>{});
+}
  

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
> 
>
  

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<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).  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?

> 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?  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....

> 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*>{});
> +}
  

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<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).  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) <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*>{});
> > +}
> 
>
  

On 9/17/22 10:31, Patrick Palka wrote:
> 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<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).  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.

I guess the problem is that a variable template specialization is 
type-dependent until it's instantiated or specialized, and here we're 
trying to instantiate if that hasn't happened yet, so using 
type_dependent_expression_p would be wrong.

The patch is OK as is.

Jason