[PATCH] c++: non-constant non-dependent decltype folding [PR104823]

[PATCH] c++: non-constant non-dependent decltype folding [PR104823]

[Patrick Palka]

instantiate_non_dependent_expr_sfinae instantiates only potentially
constant expressions, but when processing a non-dependent decltype
operand we want to instantiate it even if it's non-constant since
such non-dependent decltype is always resolved ahead of time.

Currently finish_decltype_type uses the former function, which causes us
to miss issuing a narrowing diagnostic for S{id(v)} in the below testcase
because we never instantiate this non-constant non-dependent decltype
operand.

So this patch makes finish_decltype_type use i_n_d_e_internal instead of
_sfinae.  And afterward, we need to keep processing_template_decl cleared
for sake of the later call to lvalue_kind, which handles templated and
non-templated COND_EXPR differently.  Otherwise we'd incorrectly reject
the declaration of g in cpp0x/cond2.C with:

  error: ‘g’ declared as function returning a function

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

	PR c++/104823

gcc/cp/ChangeLog:

	* semantics.cc (finish_decltype_type): Use i_n_d_e_internal
	instead of _sfinae when instantiating a non-dependent decltype
	operand, and keep processing_template_decl cleared afterward.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/Wnarrowing19.C: New test.
---
 gcc/cp/semantics.cc                       | 11 ++++++++++-
 gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C |  8 ++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C

diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 07cae993efe..66d90c2f7be 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -11217,6 +11217,8 @@ finish_decltype_type (tree expr, bool id_expression_or_member_access_p,
   /* decltype is an unevaluated context.  */
   cp_unevaluated u;
 
+  processing_template_decl_sentinel ptds (/*reset=*/false);
+
   /* Depending on the resolution of DR 1172, we may later need to distinguish
      instantiation-dependent but not type-dependent expressions so that, say,
      A<decltype(sizeof(T))>::U doesn't require 'typename'.  */
@@ -11232,7 +11234,14 @@ finish_decltype_type (tree expr, bool id_expression_or_member_access_p,
     }
   else if (processing_template_decl)
     {
-      expr = instantiate_non_dependent_expr_sfinae (expr, complain);
+      /* Instantiate the non-dependent operand to diagnose any ill-formed
+	 expressions.  And keep processing_template_decl cleared for the rest
+	 of the function (for sake of the call to lvalue_kind below, which
+	 handles templated and non-templated COND_EXPR differently).  */
+      processing_template_decl = 0;
+      /* Since we want to do this even for non-constant expressions, we
+	 use i_n_d_e_internal here instead of _sfinae.  */
+      expr = instantiate_non_dependent_expr_internal (expr, complain);
       if (expr == error_mark_node)
 	return error_mark_node;
     }
diff --git a/gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C b/gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C
new file mode 100644
index 00000000000..bd9fd2eb6f9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C
@@ -0,0 +1,8 @@
+// PR c++/104823
+// { dg-do compile { target c++11 } }
+
+struct S { S(int); };
+
+double id(double v);
+
+template<class> auto f(double v) -> decltype(S{id(v)}); // { dg-error "narrowing" }
-- 
2.35.1.354.g715d08a9e5

Re: [PATCH] c++: non-constant non-dependent decltype folding [PR104823]

[Jason Merrill]

On 3/7/22 14:41, Patrick Palka wrote:
> instantiate_non_dependent_expr_sfinae instantiates only potentially
> constant expressions

Hmm, that now strikes me as a problematic interface, as we don't know 
whether what we get back is template or non-template trees.

Maybe we want to change instantiate_non_dependent_expr to 
checking_assert that the argument is non-dependent (callers are already 
checking that), and drop the potentially-constant test?

> but when processing a non-dependent decltype
> operand we want to instantiate it even if it's non-constant since
> such non-dependent decltype is always resolved ahead of time.
> 
> Currently finish_decltype_type uses the former function, which causes us
> to miss issuing a narrowing diagnostic for S{id(v)} in the below testcase
> because we never instantiate this non-constant non-dependent decltype
> operand.
> 
> So this patch makes finish_decltype_type use i_n_d_e_internal instead of
> _sfinae.  And afterward, we need to keep processing_template_decl cleared
> for sake of the later call to lvalue_kind, which handles templated and
> non-templated COND_EXPR differently.  Otherwise we'd incorrectly reject
> the declaration of g in cpp0x/cond2.C with:
> 
>    error: ‘g’ declared as function returning a function
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> 	PR c++/104823
> 
> gcc/cp/ChangeLog:
> 
> 	* semantics.cc (finish_decltype_type): Use i_n_d_e_internal
> 	instead of _sfinae when instantiating a non-dependent decltype
> 	operand, and keep processing_template_decl cleared afterward.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/Wnarrowing19.C: New test.
> ---
>   gcc/cp/semantics.cc                       | 11 ++++++++++-
>   gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C |  8 ++++++++
>   2 files changed, 18 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C
> 
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index 07cae993efe..66d90c2f7be 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -11217,6 +11217,8 @@ finish_decltype_type (tree expr, bool id_expression_or_member_access_p,
>     /* decltype is an unevaluated context.  */
>     cp_unevaluated u;
>   
> +  processing_template_decl_sentinel ptds (/*reset=*/false);
> +
>     /* Depending on the resolution of DR 1172, we may later need to distinguish
>        instantiation-dependent but not type-dependent expressions so that, say,
>        A<decltype(sizeof(T))>::U doesn't require 'typename'.  */
> @@ -11232,7 +11234,14 @@ finish_decltype_type (tree expr, bool id_expression_or_member_access_p,
>       }
>     else if (processing_template_decl)
>       {
> -      expr = instantiate_non_dependent_expr_sfinae (expr, complain);
> +      /* Instantiate the non-dependent operand to diagnose any ill-formed
> +	 expressions.  And keep processing_template_decl cleared for the rest
> +	 of the function (for sake of the call to lvalue_kind below, which
> +	 handles templated and non-templated COND_EXPR differently).  */
> +      processing_template_decl = 0;
> +      /* Since we want to do this even for non-constant expressions, we
> +	 use i_n_d_e_internal here instead of _sfinae.  */
> +      expr = instantiate_non_dependent_expr_internal (expr, complain);
>         if (expr == error_mark_node)
>   	return error_mark_node;
>       }
> diff --git a/gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C b/gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C
> new file mode 100644
> index 00000000000..bd9fd2eb6f9
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C
> @@ -0,0 +1,8 @@
> +// PR c++/104823
> +// { dg-do compile { target c++11 } }
> +
> +struct S { S(int); };
> +
> +double id(double v);
> +
> +template<class> auto f(double v) -> decltype(S{id(v)}); // { dg-error "narrowing" }

Re: [PATCH] c++: non-constant non-dependent decltype folding [PR104823]

[Patrick Palka]

On Mon, 7 Mar 2022, Jason Merrill wrote:

> On 3/7/22 14:41, Patrick Palka wrote:
> > instantiate_non_dependent_expr_sfinae instantiates only potentially
> > constant expressions
> 
> Hmm, that now strikes me as a problematic interface, as we don't know whether
> what we get back is template or non-template trees.
> 
> Maybe we want to change instantiate_non_dependent_expr to checking_assert that
> the argument is non-dependent (callers are already checking that), and drop
> the potentially-constant test?

That sounds like a nice improvement.  But it happens to break

  template<int N> using type = decltype(N);

beause finish_decltype_type checks instantiation_dependent_uneval_expression_p
(which is false here) instead of instantiation_dependent_expression_p
(which is true here) before calling instantiate_non_dependent_expr, so
we end up tripping over the proposed checking_assert (which checks the
latter stronger form of dependence).

I suspect other callers of instantiate_non_dependent_expr might have a
similar problem if they use a weaker dependence check than
instantiation_dependent_expression_p, e.g. build_noexcept_spec only
checks value_dependent_expression_p.

I wonder if we should relax the proposed checking_assert in i_n_d_e, or
strengthen the dependence checks performed by its callers, or something
else?

Here's the diff I'm working with:

-- >8 --

 gcc/cp/parser.cc    |  2 +-
 gcc/cp/pt.cc        | 21 ++++++---------------
 gcc/cp/semantics.cc | 11 ++++-------
 3 files changed, 11 insertions(+), 23 deletions(-)

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 20aab5eb6b1..a570a9163b9 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -7986,7 +7986,7 @@ cp_parser_parenthesized_expression_list_elt (cp_parser *parser, bool cast_p,
     expr = cp_parser_assignment_expression (parser, /*pidk=*/NULL, cast_p);
 
   if (fold_expr_p)
-    expr = instantiate_non_dependent_expr (expr);
+    expr = fold_non_dependent_expr (expr);
 
   /* If we have an ellipsis, then this is an expression expansion.  */
   if (allow_expansion_p
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 53a74636279..1b2d9a7e4b1 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -6350,8 +6350,7 @@ redeclare_class_template (tree type, tree parms, tree cons)
 /* The actual substitution part of instantiate_non_dependent_expr_sfinae,
    to be used when the caller has already checked
    (processing_template_decl
-    && !instantiation_dependent_expression_p (expr)
-    && potential_constant_expression (expr))
+    && !instantiation_dependent_expression_p (expr))
    and cleared processing_template_decl.  */
 
 tree
@@ -6365,8 +6364,7 @@ instantiate_non_dependent_expr_internal (tree expr, tsubst_flags_t complain)
 				/*integral_constant_expression_p=*/true);
 }
 
-/* Simplify EXPR if it is a non-dependent expression.  Returns the
-   (possibly simplified) expression.  */
+/* Instantiate the non-dependent expression EXPR.  */
 
 tree
 instantiate_non_dependent_expr_sfinae (tree expr, tsubst_flags_t complain)
@@ -6374,16 +6372,9 @@ instantiate_non_dependent_expr_sfinae (tree expr, tsubst_flags_t complain)
   if (expr == NULL_TREE)
     return NULL_TREE;
 
-  /* If we're in a template, but EXPR isn't value dependent, simplify
-     it.  We're supposed to treat:
-
-       template <typename T> void f(T[1 + 1]);
-       template <typename T> void f(T[2]);
-
-     as two declarations of the same function, for example.  */
-  if (processing_template_decl
-      && is_nondependent_constant_expression (expr))
+  if (processing_template_decl)
     {
+      gcc_checking_assert (!instantiation_dependent_expression_p (expr));
       processing_template_decl_sentinel s;
       expr = instantiate_non_dependent_expr_internal (expr, complain);
     }
@@ -6396,8 +6387,8 @@ instantiate_non_dependent_expr (tree expr)
   return instantiate_non_dependent_expr_sfinae (expr, tf_error);
 }
 
-/* Like instantiate_non_dependent_expr, but return NULL_TREE rather than
-   an uninstantiated expression.  */
+/* Like instantiate_non_dependent_expr, but return NULL_TREE if the
+   expression is dependent or non-constant.  */
 
 tree
 instantiate_non_dependent_or_null (tree expr)
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 66d90c2f7be..8f744eb21b6 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -11234,16 +11234,13 @@ finish_decltype_type (tree expr, bool id_expression_or_member_access_p,
     }
   else if (processing_template_decl)
     {
-      /* Instantiate the non-dependent operand to diagnose any ill-formed
-	 expressions.  And keep processing_template_decl cleared for the rest
+      expr = instantiate_non_dependent_expr_sfinae (expr, complain);
+      if (expr == error_mark_node)
+	return error_mark_node;
+      /* Keep processing_template_decl cleared for the rest
 	 of the function (for sake of the call to lvalue_kind below, which
 	 handles templated and non-templated COND_EXPR differently).  */
       processing_template_decl = 0;
-      /* Since we want to do this even for non-constant expressions, we
-	 use i_n_d_e_internal here instead of _sfinae.  */
-      expr = instantiate_non_dependent_expr_internal (expr, complain);
-      if (expr == error_mark_node)
-	return error_mark_node;
     }
 
   /* The type denoted by decltype(e) is defined as follows:  */

Re: [PATCH] c++: non-constant non-dependent decltype folding [PR104823]

[Jason Merrill]

On 3/8/22 12:54, Patrick Palka wrote:
> 
> 
> On Mon, 7 Mar 2022, Jason Merrill wrote:
> 
>> On 3/7/22 14:41, Patrick Palka wrote:
>>> instantiate_non_dependent_expr_sfinae instantiates only potentially
>>> constant expressions
>>
>> Hmm, that now strikes me as a problematic interface, as we don't know whether
>> what we get back is template or non-template trees.
>>
>> Maybe we want to change instantiate_non_dependent_expr to checking_assert that
>> the argument is non-dependent (callers are already checking that), and drop
>> the potentially-constant test?
> 
> That sounds like a nice improvement.  But it happens to break
> 
>    template<int N> using type = decltype(N);
> 
> beause finish_decltype_type checks instantiation_dependent_uneval_expression_p
> (which is false here) instead of instantiation_dependent_expression_p
> (which is true here) before calling instantiate_non_dependent_expr, so
> we end up tripping over the proposed checking_assert (which checks the
> latter stronger form of dependence).
> 
> I suspect other callers of instantiate_non_dependent_expr might have a
> similar problem if they use a weaker dependence check than
> instantiation_dependent_expression_p, e.g. build_noexcept_spec only
> checks value_dependent_expression_p.
> 
> I wonder if we should relax the proposed checking_assert in i_n_d_e, or
> strengthen the dependence checks performed by its callers, or something
> else?

I think relax the assert to _uneval and strengthen callers that use 
value_dep.

> Here's the diff I'm working with:
> 
> -- >8 --
> 
>   gcc/cp/parser.cc    |  2 +-
>   gcc/cp/pt.cc        | 21 ++++++---------------
>   gcc/cp/semantics.cc | 11 ++++-------
>   3 files changed, 11 insertions(+), 23 deletions(-)
> 
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index 20aab5eb6b1..a570a9163b9 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -7986,7 +7986,7 @@ cp_parser_parenthesized_expression_list_elt (cp_parser *parser, bool cast_p,
>       expr = cp_parser_assignment_expression (parser, /*pidk=*/NULL, cast_p);
>   
>     if (fold_expr_p)
> -    expr = instantiate_non_dependent_expr (expr);
> +    expr = fold_non_dependent_expr (expr);
>   
>     /* If we have an ellipsis, then this is an expression expansion.  */
>     if (allow_expansion_p
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 53a74636279..1b2d9a7e4b1 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -6350,8 +6350,7 @@ redeclare_class_template (tree type, tree parms, tree cons)
>   /* The actual substitution part of instantiate_non_dependent_expr_sfinae,
>      to be used when the caller has already checked
>      (processing_template_decl
> -    && !instantiation_dependent_expression_p (expr)
> -    && potential_constant_expression (expr))
> +    && !instantiation_dependent_expression_p (expr))
>      and cleared processing_template_decl.  */
>   
>   tree
> @@ -6365,8 +6364,7 @@ instantiate_non_dependent_expr_internal (tree expr, tsubst_flags_t complain)
>   				/*integral_constant_expression_p=*/true);
>   }
>   
> -/* Simplify EXPR if it is a non-dependent expression.  Returns the
> -   (possibly simplified) expression.  */
> +/* Instantiate the non-dependent expression EXPR.  */
>   
>   tree
>   instantiate_non_dependent_expr_sfinae (tree expr, tsubst_flags_t complain)
> @@ -6374,16 +6372,9 @@ instantiate_non_dependent_expr_sfinae (tree expr, tsubst_flags_t complain)
>     if (expr == NULL_TREE)
>       return NULL_TREE;
>   
> -  /* If we're in a template, but EXPR isn't value dependent, simplify
> -     it.  We're supposed to treat:
> -
> -       template <typename T> void f(T[1 + 1]);
> -       template <typename T> void f(T[2]);
> -
> -     as two declarations of the same function, for example.  */
> -  if (processing_template_decl
> -      && is_nondependent_constant_expression (expr))
> +  if (processing_template_decl)
>       {
> +      gcc_checking_assert (!instantiation_dependent_expression_p (expr));
>         processing_template_decl_sentinel s;
>         expr = instantiate_non_dependent_expr_internal (expr, complain);
>       }
> @@ -6396,8 +6387,8 @@ instantiate_non_dependent_expr (tree expr)
>     return instantiate_non_dependent_expr_sfinae (expr, tf_error);
>   }
>   
> -/* Like instantiate_non_dependent_expr, but return NULL_TREE rather than
> -   an uninstantiated expression.  */
> +/* Like instantiate_non_dependent_expr, but return NULL_TREE if the
> +   expression is dependent or non-constant.  */
>   
>   tree
>   instantiate_non_dependent_or_null (tree expr)
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index 66d90c2f7be..8f744eb21b6 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -11234,16 +11234,13 @@ finish_decltype_type (tree expr, bool id_expression_or_member_access_p,
>       }
>     else if (processing_template_decl)
>       {
> -      /* Instantiate the non-dependent operand to diagnose any ill-formed
> -	 expressions.  And keep processing_template_decl cleared for the rest
> +      expr = instantiate_non_dependent_expr_sfinae (expr, complain);
> +      if (expr == error_mark_node)
> +	return error_mark_node;
> +      /* Keep processing_template_decl cleared for the rest
>   	 of the function (for sake of the call to lvalue_kind below, which
>   	 handles templated and non-templated COND_EXPR differently).  */
>         processing_template_decl = 0;
> -      /* Since we want to do this even for non-constant expressions, we
> -	 use i_n_d_e_internal here instead of _sfinae.  */
> -      expr = instantiate_non_dependent_expr_internal (expr, complain);
> -      if (expr == error_mark_node)
> -	return error_mark_node;
>       }
>   
>     /* The type denoted by decltype(e) is defined as follows:  */
> 

Re: [PATCH] c++: non-constant non-dependent decltype folding [PR104823]

[Patrick Palka]

On Tue, 8 Mar 2022, Jason Merrill wrote:

> On 3/8/22 12:54, Patrick Palka wrote:
> > 
> > 
> > On Mon, 7 Mar 2022, Jason Merrill wrote:
> > 
> > > On 3/7/22 14:41, Patrick Palka wrote:
> > > > instantiate_non_dependent_expr_sfinae instantiates only potentially
> > > > constant expressions
> > > 
> > > Hmm, that now strikes me as a problematic interface, as we don't know
> > > whether
> > > what we get back is template or non-template trees.
> > > 
> > > Maybe we want to change instantiate_non_dependent_expr to checking_assert
> > > that
> > > the argument is non-dependent (callers are already checking that), and
> > > drop
> > > the potentially-constant test?
> > 
> > That sounds like a nice improvement.  But it happens to break
> > 
> >    template<int N> using type = decltype(N);
> > 
> > beause finish_decltype_type checks
> > instantiation_dependent_uneval_expression_p
> > (which is false here) instead of instantiation_dependent_expression_p
> > (which is true here) before calling instantiate_non_dependent_expr, so
> > we end up tripping over the proposed checking_assert (which checks the
> > latter stronger form of dependence).
> > 
> > I suspect other callers of instantiate_non_dependent_expr might have a
> > similar problem if they use a weaker dependence check than
> > instantiation_dependent_expression_p, e.g. build_noexcept_spec only
> > checks value_dependent_expression_p.
> > 
> > I wonder if we should relax the proposed checking_assert in i_n_d_e, or
> > strengthen the dependence checks performed by its callers, or something
> > else?
> 
> I think relax the assert to _uneval and strengthen callers that use value_dep.

Sounds good, like so?  Note this patch doesn't touch
instantiate_non_dependent_or_null or fold_non_dependent_expr, since the
former already never returns a templated tree, and callers of the latter
should only care about the constant-ness not template-ness of the result
IIUC.

Boostrapped and regtested on x86_64-pc-linux-gnu.

-- >8 --

Subject: [PATCH] c++: non-constant non-dependent decltype folding [PR104823]

When processing a non-dependent decltype operand we want to instantiate
it even if it's non-constant since non-dependent decltype is always
resolved ahead of time.  But currently finish_decltype_type uses
instantiate_non_dependent_expr, which instantiates only potentially
constant expressions, and this causes us to miss diagnosing the narrowing
conversion in S{id(v)} in the below testcase because we never instantiate
this non-constant non-dependent decltype operand.

In light of

  > On Mon, 7 Mar 2022, Jason Merrill wrote:
  >> On 3/7/22 14:41, Patrick Palka wrote:
  >>> instantiate_non_dependent_expr instantiates only potentially constant
  >>> expressions
  >>
  >> Hmm, that now strikes me as a problematic interface, as we don't know whether
  >> what we get back is template or non-template trees.

this patch drops the potentially-constant check in i_n_d_e, and turns
its dependence check into a checking_assert, since most callers already
check that the argument is non-dependent.  This patch also relaxes the
dependence check in i_n_d_e to use the _uneval version and strengthens
the dependence checks used by callers accordingly.

In cp_parser_parenthesized_expression_list_elt we were calling
instantiate_non_dependent_expr without first checking for non-dependence.
We could fix this by guarding the call appropriately, but I noticed we
also fold non-dependent attributes later from cp_check_const_attribute.
This double instantiation causes us to reject constexpr-attribute4.C
below due to the second folding seeing non-templated trees (an existing
bug).  Thus the right solution here seems to be to remove this unguarded
call to i_n_d_e so that we end up folding non-dependent attributes only
once.

Finally, after calling i_n_d_e in finish_decltype_type we need to keep
processing_template_decl cleared for sake of the later call to
lvalue_kind, which handles templated and non-templated COND_EXPR
differently.  Otherwise we'd incorrectly reject the declaration of g in
cpp0x/cond2.C with:

  error: 'g' declared as function returning a function

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

	PR c++/104823

gcc/cp/ChangeLog:

	* except.cc (build_noexcept_spec): Strengthen dependence check
	to instantiation_dependent_expression_p.
	* parser.cc (cp_parser_parenthesized_expression_list_elt):
	Remove fold_expr_p parameter and call
	instantiate_non_dependent_expr.
	(cp_parser_parenthesized_expression_list): Adjust accordingly.
	* pt.cc (expand_integer_pack): Strengthen dependence check
	to instantiation_dependent_expression_p.
	(instantiate_non_dependent_expr_internal): Adjust comment.
	(instantiate_non_dependent_expr_sfinae): Likewise.  Drop
	the potentially-constant check, and relax and turn the
	dependence check into a checking assert.
	(instantiate_non_dependent_or_null): Adjust comment.
	* semantics.cc (finish_decltype_type): Keep
	processing_template_decl cleared after calling
	instantiate_non_dependent_expr_sfinae.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/Wnarrowing19.C: New test.
---
 gcc/cp/except.cc                              |  2 +-
 gcc/cp/parser.cc                              |  8 -------
 gcc/cp/pt.cc                                  | 24 ++++++-------------
 gcc/cp/semantics.cc                           |  6 +++++
 gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C     |  8 +++++++
 .../g++.dg/cpp0x/constexpr-attribute4.C       | 14 +++++++++++
 6 files changed, 36 insertions(+), 26 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-attribute4.C

diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc
index 9b746be231a..da0a65c613d 100644
--- a/gcc/cp/except.cc
+++ b/gcc/cp/except.cc
@@ -1253,7 +1253,7 @@ build_noexcept_spec (tree expr, tsubst_flags_t complain)
   if (check_for_bare_parameter_packs (expr))
     return error_mark_node;
   if (TREE_CODE (expr) != DEFERRED_NOEXCEPT
-      && !value_dependent_expression_p (expr))
+      && !instantiation_dependent_expression_p (expr))
     {
       expr = build_converted_constant_bool_expr (expr, complain);
       expr = instantiate_non_dependent_expr_sfinae (expr, complain);
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 20aab5eb6b1..c9de8e8d050 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -7958,7 +7958,6 @@ cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p,
 static cp_expr
 cp_parser_parenthesized_expression_list_elt (cp_parser *parser, bool cast_p,
 					     bool allow_expansion_p,
-					     bool fold_expr_p,
 					     bool *non_constant_p)
 {
   cp_expr expr (NULL_TREE);
@@ -7985,9 +7984,6 @@ cp_parser_parenthesized_expression_list_elt (cp_parser *parser, bool cast_p,
   else
     expr = cp_parser_assignment_expression (parser, /*pidk=*/NULL, cast_p);
 
-  if (fold_expr_p)
-    expr = instantiate_non_dependent_expr (expr);
-
   /* If we have an ellipsis, then this is an expression expansion.  */
   if (allow_expansion_p
       && cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS))
@@ -8053,8 +8049,6 @@ cp_parser_postfix_open_square_expression (cp_parser *parser,
 							       false,
 							       /*allow_exp_p=*/
 							       true,
-							       /*fold_expr_p=*/
-							       false,
 							       /*non_cst_p=*/
 							       NULL);
 
@@ -8424,7 +8418,6 @@ cp_parser_parenthesized_expression_list (cp_parser* parser,
 					 bool wrap_locations_p)
 {
   vec<tree, va_gc> *expression_list;
-  bool fold_expr_p = is_attribute_list != non_attr;
   tree identifier = NULL_TREE;
   bool saved_greater_than_is_operator_p;
 
@@ -8467,7 +8460,6 @@ cp_parser_parenthesized_expression_list (cp_parser* parser,
 	    expr
 	      = cp_parser_parenthesized_expression_list_elt (parser, cast_p,
 							     allow_expansion_p,
-							     fold_expr_p,
 							     non_constant_p);
 
 	    if (wrap_locations_p)
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index e8b5d8fbb73..75b43ba60fe 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -3817,7 +3817,7 @@ expand_integer_pack (tree call, tree args, tsubst_flags_t complain,
   tree hi = tsubst_copy_and_build (ohi, args, complain, in_decl,
 				   false/*fn*/, true/*int_cst*/);
 
-  if (value_dependent_expression_p (hi))
+  if (instantiation_dependent_expression_p (hi))
     {
       if (hi != ohi)
 	{
@@ -6349,9 +6349,7 @@ redeclare_class_template (tree type, tree parms, tree cons)
 
 /* The actual substitution part of instantiate_non_dependent_expr_sfinae,
    to be used when the caller has already checked
-   (processing_template_decl
-    && !instantiation_dependent_expression_p (expr)
-    && potential_constant_expression (expr))
+    !instantiation_dependent_uneval_expression_p (expr)
    and cleared processing_template_decl.  */
 
 tree
@@ -6365,8 +6363,7 @@ instantiate_non_dependent_expr_internal (tree expr, tsubst_flags_t complain)
 				/*integral_constant_expression_p=*/true);
 }
 
-/* Simplify EXPR if it is a non-dependent expression.  Returns the
-   (possibly simplified) expression.  */
+/* Instantiate the non-dependent expression EXPR.  */
 
 tree
 instantiate_non_dependent_expr_sfinae (tree expr, tsubst_flags_t complain)
@@ -6374,16 +6371,9 @@ instantiate_non_dependent_expr_sfinae (tree expr, tsubst_flags_t complain)
   if (expr == NULL_TREE)
     return NULL_TREE;
 
-  /* If we're in a template, but EXPR isn't value dependent, simplify
-     it.  We're supposed to treat:
-
-       template <typename T> void f(T[1 + 1]);
-       template <typename T> void f(T[2]);
-
-     as two declarations of the same function, for example.  */
-  if (processing_template_decl
-      && is_nondependent_constant_expression (expr))
+  if (processing_template_decl)
     {
+      gcc_checking_assert (!instantiation_dependent_uneval_expression_p (expr));
       processing_template_decl_sentinel s;
       expr = instantiate_non_dependent_expr_internal (expr, complain);
     }
@@ -6396,8 +6386,8 @@ instantiate_non_dependent_expr (tree expr)
   return instantiate_non_dependent_expr_sfinae (expr, tf_error);
 }
 
-/* Like instantiate_non_dependent_expr, but return NULL_TREE rather than
-   an uninstantiated expression.  */
+/* Like instantiate_non_dependent_expr, but return NULL_TREE if the
+   expression is dependent or non-constant.  */
 
 tree
 instantiate_non_dependent_or_null (tree expr)
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 07cae993efe..773a83eec3d 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -11217,6 +11217,8 @@ finish_decltype_type (tree expr, bool id_expression_or_member_access_p,
   /* decltype is an unevaluated context.  */
   cp_unevaluated u;
 
+  processing_template_decl_sentinel ptds (/*reset=*/false);
+
   /* Depending on the resolution of DR 1172, we may later need to distinguish
      instantiation-dependent but not type-dependent expressions so that, say,
      A<decltype(sizeof(T))>::U doesn't require 'typename'.  */
@@ -11235,6 +11237,10 @@ finish_decltype_type (tree expr, bool id_expression_or_member_access_p,
       expr = instantiate_non_dependent_expr_sfinae (expr, complain);
       if (expr == error_mark_node)
 	return error_mark_node;
+      /* Keep processing_template_decl cleared for the rest of the function
+	 (for sake of the call to lvalue_kind below, which handles templated
+	 and non-templated COND_EXPR differently).  */
+      processing_template_decl = 0;
     }
 
   /* The type denoted by decltype(e) is defined as follows:  */
diff --git a/gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C b/gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C
new file mode 100644
index 00000000000..bd9fd2eb6f9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C
@@ -0,0 +1,8 @@
+// PR c++/104823
+// { dg-do compile { target c++11 } }
+
+struct S { S(int); };
+
+double id(double v);
+
+template<class> auto f(double v) -> decltype(S{id(v)}); // { dg-error "narrowing" }
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-attribute4.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-attribute4.C
new file mode 100644
index 00000000000..2cff6d333f9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-attribute4.C
@@ -0,0 +1,14 @@
+// Verify we correctly handle the non-dependent attribute expression which,
+// if we were to instantiate it twice, would result in a bogus error.
+// { dg-do compile { target { c++11 } } }
+
+struct A {
+  constexpr int f() const { return __alignof__(int); };
+};
+
+template<class...>
+void f() {
+  int a __attribute__((aligned(A{}.f())));
+}
+
+template void f();
-- 
2.35.1.415.gc2162907e9

Re: [PATCH] c++: non-constant non-dependent decltype folding [PR104823]

[Jason Merrill]

On 3/8/22 16:57, Patrick Palka wrote:
> On Tue, 8 Mar 2022, Jason Merrill wrote:
> 
>> On 3/8/22 12:54, Patrick Palka wrote:
>>>
>>>
>>> On Mon, 7 Mar 2022, Jason Merrill wrote:
>>>
>>>> On 3/7/22 14:41, Patrick Palka wrote:
>>>>> instantiate_non_dependent_expr_sfinae instantiates only potentially
>>>>> constant expressions
>>>>
>>>> Hmm, that now strikes me as a problematic interface, as we don't know
>>>> whether
>>>> what we get back is template or non-template trees.
>>>>
>>>> Maybe we want to change instantiate_non_dependent_expr to checking_assert
>>>> that
>>>> the argument is non-dependent (callers are already checking that), and
>>>> drop
>>>> the potentially-constant test?
>>>
>>> That sounds like a nice improvement.  But it happens to break
>>>
>>>     template<int N> using type = decltype(N);
>>>
>>> beause finish_decltype_type checks
>>> instantiation_dependent_uneval_expression_p
>>> (which is false here) instead of instantiation_dependent_expression_p
>>> (which is true here) before calling instantiate_non_dependent_expr, so
>>> we end up tripping over the proposed checking_assert (which checks the
>>> latter stronger form of dependence).
>>>
>>> I suspect other callers of instantiate_non_dependent_expr might have a
>>> similar problem if they use a weaker dependence check than
>>> instantiation_dependent_expression_p, e.g. build_noexcept_spec only
>>> checks value_dependent_expression_p.
>>>
>>> I wonder if we should relax the proposed checking_assert in i_n_d_e, or
>>> strengthen the dependence checks performed by its callers, or something
>>> else?
>>
>> I think relax the assert to _uneval and strengthen callers that use value_dep.
> 
> Sounds good, like so?  Note this patch doesn't touch
> instantiate_non_dependent_or_null or fold_non_dependent_expr, since the
> former already never returns a templated tree, and callers of the latter
> should only care about the constant-ness not template-ness of the result
> IIUC.
> 
> Boostrapped and regtested on x86_64-pc-linux-gnu.
> 
> -- >8 --
> 
> Subject: [PATCH] c++: non-constant non-dependent decltype folding [PR104823]
> 
> When processing a non-dependent decltype operand we want to instantiate
> it even if it's non-constant since non-dependent decltype is always
> resolved ahead of time.  But currently finish_decltype_type uses
> instantiate_non_dependent_expr, which instantiates only potentially
> constant expressions, and this causes us to miss diagnosing the narrowing
> conversion in S{id(v)} in the below testcase because we never instantiate
> this non-constant non-dependent decltype operand.
> 
> In light of
> 
>    > On Mon, 7 Mar 2022, Jason Merrill wrote:
>    >> On 3/7/22 14:41, Patrick Palka wrote:
>    >>> instantiate_non_dependent_expr instantiates only potentially constant
>    >>> expressions
>    >>
>    >> Hmm, that now strikes me as a problematic interface, as we don't know whether
>    >> what we get back is template or non-template trees.
> 
> this patch drops the potentially-constant check in i_n_d_e, and turns
> its dependence check into a checking_assert, since most callers already
> check that the argument is non-dependent.  This patch also relaxes the
> dependence check in i_n_d_e to use the _uneval version and strengthens
> the dependence checks used by callers accordingly.
> 
> In cp_parser_parenthesized_expression_list_elt we were calling
> instantiate_non_dependent_expr without first checking for non-dependence.
> We could fix this by guarding the call appropriately, but I noticed we
> also fold non-dependent attributes later from cp_check_const_attribute.
> This double instantiation causes us to reject constexpr-attribute4.C
> below due to the second folding seeing non-templated trees (an existing
> bug).  Thus the right solution here seems to be to remove this unguarded
> call to i_n_d_e so that we end up folding non-dependent attributes only
> once.
> 
> Finally, after calling i_n_d_e in finish_decltype_type we need to keep
> processing_template_decl cleared for sake of the later call to
> lvalue_kind, which handles templated and non-templated COND_EXPR
> differently.  Otherwise we'd incorrectly reject the declaration of g in
> cpp0x/cond2.C with:
> 
>    error: 'g' declared as function returning a function
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> 	PR c++/104823
> 
> gcc/cp/ChangeLog:
> 
> 	* except.cc (build_noexcept_spec): Strengthen dependence check
> 	to instantiation_dependent_expression_p.
> 	* parser.cc (cp_parser_parenthesized_expression_list_elt):
> 	Remove fold_expr_p parameter and call
> 	instantiate_non_dependent_expr.
> 	(cp_parser_parenthesized_expression_list): Adjust accordingly.
> 	* pt.cc (expand_integer_pack): Strengthen dependence check
> 	to instantiation_dependent_expression_p.
> 	(instantiate_non_dependent_expr_internal): Adjust comment.
> 	(instantiate_non_dependent_expr_sfinae): Likewise.  Drop
> 	the potentially-constant check, and relax and turn the
> 	dependence check into a checking assert.
> 	(instantiate_non_dependent_or_null): Adjust comment.
> 	* semantics.cc (finish_decltype_type): Keep
> 	processing_template_decl cleared after calling
> 	instantiate_non_dependent_expr_sfinae.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/Wnarrowing19.C: New test.
> ---
>   gcc/cp/except.cc                              |  2 +-
>   gcc/cp/parser.cc                              |  8 -------
>   gcc/cp/pt.cc                                  | 24 ++++++-------------
>   gcc/cp/semantics.cc                           |  6 +++++
>   gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C     |  8 +++++++
>   .../g++.dg/cpp0x/constexpr-attribute4.C       | 14 +++++++++++
>   6 files changed, 36 insertions(+), 26 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-attribute4.C
> 
> diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc
> index 9b746be231a..da0a65c613d 100644
> --- a/gcc/cp/except.cc
> +++ b/gcc/cp/except.cc
> @@ -1253,7 +1253,7 @@ build_noexcept_spec (tree expr, tsubst_flags_t complain)
>     if (check_for_bare_parameter_packs (expr))
>       return error_mark_node;
>     if (TREE_CODE (expr) != DEFERRED_NOEXCEPT
> -      && !value_dependent_expression_p (expr))
> +      && !instantiation_dependent_expression_p (expr))
>       {
>         expr = build_converted_constant_bool_expr (expr, complain);
>         expr = instantiate_non_dependent_expr_sfinae (expr, complain);
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index 20aab5eb6b1..c9de8e8d050 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -7958,7 +7958,6 @@ cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p,
>   static cp_expr
>   cp_parser_parenthesized_expression_list_elt (cp_parser *parser, bool cast_p,
>   					     bool allow_expansion_p,
> -					     bool fold_expr_p,
>   					     bool *non_constant_p)
>   {
>     cp_expr expr (NULL_TREE);
> @@ -7985,9 +7984,6 @@ cp_parser_parenthesized_expression_list_elt (cp_parser *parser, bool cast_p,
>     else
>       expr = cp_parser_assignment_expression (parser, /*pidk=*/NULL, cast_p);
>   
> -  if (fold_expr_p)
> -    expr = instantiate_non_dependent_expr (expr);
> -
>     /* If we have an ellipsis, then this is an expression expansion.  */
>     if (allow_expansion_p
>         && cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS))
> @@ -8053,8 +8049,6 @@ cp_parser_postfix_open_square_expression (cp_parser *parser,
>   							       false,
>   							       /*allow_exp_p=*/
>   							       true,
> -							       /*fold_expr_p=*/
> -							       false,
>   							       /*non_cst_p=*/
>   							       NULL);
>   
> @@ -8424,7 +8418,6 @@ cp_parser_parenthesized_expression_list (cp_parser* parser,
>   					 bool wrap_locations_p)
>   {
>     vec<tree, va_gc> *expression_list;
> -  bool fold_expr_p = is_attribute_list != non_attr;
>     tree identifier = NULL_TREE;
>     bool saved_greater_than_is_operator_p;
>   
> @@ -8467,7 +8460,6 @@ cp_parser_parenthesized_expression_list (cp_parser* parser,
>   	    expr
>   	      = cp_parser_parenthesized_expression_list_elt (parser, cast_p,
>   							     allow_expansion_p,
> -							     fold_expr_p,
>   							     non_constant_p);
>   
>   	    if (wrap_locations_p)
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index e8b5d8fbb73..75b43ba60fe 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -3817,7 +3817,7 @@ expand_integer_pack (tree call, tree args, tsubst_flags_t complain,
>     tree hi = tsubst_copy_and_build (ohi, args, complain, in_decl,
>   				   false/*fn*/, true/*int_cst*/);
>   
> -  if (value_dependent_expression_p (hi))
> +  if (instantiation_dependent_expression_p (hi))
>       {
>         if (hi != ohi)
>   	{
> @@ -6349,9 +6349,7 @@ redeclare_class_template (tree type, tree parms, tree cons)
>   
>   /* The actual substitution part of instantiate_non_dependent_expr_sfinae,
>      to be used when the caller has already checked
> -   (processing_template_decl
> -    && !instantiation_dependent_expression_p (expr)
> -    && potential_constant_expression (expr))
> +    !instantiation_dependent_uneval_expression_p (expr)
>      and cleared processing_template_decl.  */
>   
>   tree
> @@ -6365,8 +6363,7 @@ instantiate_non_dependent_expr_internal (tree expr, tsubst_flags_t complain)
>   				/*integral_constant_expression_p=*/true);
>   }
>   
> -/* Simplify EXPR if it is a non-dependent expression.  Returns the
> -   (possibly simplified) expression.  */
> +/* Instantiate the non-dependent expression EXPR.  */
>   
>   tree
>   instantiate_non_dependent_expr_sfinae (tree expr, tsubst_flags_t complain)
> @@ -6374,16 +6371,9 @@ instantiate_non_dependent_expr_sfinae (tree expr, tsubst_flags_t complain)
>     if (expr == NULL_TREE)
>       return NULL_TREE;
>   
> -  /* If we're in a template, but EXPR isn't value dependent, simplify
> -     it.  We're supposed to treat:
> -
> -       template <typename T> void f(T[1 + 1]);
> -       template <typename T> void f(T[2]);
> -
> -     as two declarations of the same function, for example.  */
> -  if (processing_template_decl
> -      && is_nondependent_constant_expression (expr))
> +  if (processing_template_decl)
>       {
> +      gcc_checking_assert (!instantiation_dependent_uneval_expression_p (expr));

Let's add a comment that the caller should have checked this already. 
OK with that change.

>         processing_template_decl_sentinel s;
>         expr = instantiate_non_dependent_expr_internal (expr, complain);
>       }
> @@ -6396,8 +6386,8 @@ instantiate_non_dependent_expr (tree expr)
>     return instantiate_non_dependent_expr_sfinae (expr, tf_error);
>   }
>   
> -/* Like instantiate_non_dependent_expr, but return NULL_TREE rather than
> -   an uninstantiated expression.  */
> +/* Like instantiate_non_dependent_expr, but return NULL_TREE if the
> +   expression is dependent or non-constant.  */
>   
>   tree
>   instantiate_non_dependent_or_null (tree expr)
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index 07cae993efe..773a83eec3d 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -11217,6 +11217,8 @@ finish_decltype_type (tree expr, bool id_expression_or_member_access_p,
>     /* decltype is an unevaluated context.  */
>     cp_unevaluated u;
>   
> +  processing_template_decl_sentinel ptds (/*reset=*/false);
> +
>     /* Depending on the resolution of DR 1172, we may later need to distinguish
>        instantiation-dependent but not type-dependent expressions so that, say,
>        A<decltype(sizeof(T))>::U doesn't require 'typename'.  */
> @@ -11235,6 +11237,10 @@ finish_decltype_type (tree expr, bool id_expression_or_member_access_p,
>         expr = instantiate_non_dependent_expr_sfinae (expr, complain);
>         if (expr == error_mark_node)
>   	return error_mark_node;
> +      /* Keep processing_template_decl cleared for the rest of the function
> +	 (for sake of the call to lvalue_kind below, which handles templated
> +	 and non-templated COND_EXPR differently).  */
> +      processing_template_decl = 0;
>       }
>   
>     /* The type denoted by decltype(e) is defined as follows:  */
> diff --git a/gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C b/gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C
> new file mode 100644
> index 00000000000..bd9fd2eb6f9
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/Wnarrowing19.C
> @@ -0,0 +1,8 @@
> +// PR c++/104823
> +// { dg-do compile { target c++11 } }
> +
> +struct S { S(int); };
> +
> +double id(double v);
> +
> +template<class> auto f(double v) -> decltype(S{id(v)}); // { dg-error "narrowing" }
> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-attribute4.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-attribute4.C
> new file mode 100644
> index 00000000000..2cff6d333f9
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-attribute4.C
> @@ -0,0 +1,14 @@
> +// Verify we correctly handle the non-dependent attribute expression which,
> +// if we were to instantiate it twice, would result in a bogus error.
> +// { dg-do compile { target { c++11 } } }
> +
> +struct A {
> +  constexpr int f() const { return __alignof__(int); };
> +};
> +
> +template<class...>
> +void f() {
> +  int a __attribute__((aligned(A{}.f())));
> +}
> +
> +template void f();