[PATCH v2] c++: Prevent warnings for value-dependent exprs [PR96742]

[Marek Polacek]

On Wed, Oct 28, 2020 at 05:48:08PM -0400, Jason Merrill wrote:
> On 10/28/20 5:29 PM, Marek Polacek wrote:
> > On Wed, Oct 28, 2020 at 02:46:36PM -0400, Jason Merrill wrote:
> > > On 10/28/20 2:00 PM, Marek Polacek wrote:
> > > > On Tue, Oct 27, 2020 at 01:36:30PM -0400, Jason Merrill wrote:
> > > > > On 10/24/20 6:52 PM, Marek Polacek wrote:
> > > > > > Here, in r11-155, I changed the call to uses_template_parms to
> > > > > > type_dependent_expression_p_push to avoid a crash in C++98 in
> > > > > > value_dependent_expression_p on a non-constant expression.  But that
> > > > > > prompted a host of complaints that we now warn for value-dependent
> > > > > > expressions in templates.  Those warnings are technically valid, but
> > > > > > people still don't want them because they're awkward to avoid.  So let's
> > > > > > partially revert my earlier fix and make sure that we don't ICE in
> > > > > > value_dependent_expression_p by checking potential_constant_expression
> > > > > > first.
> > > > > > 
> > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?
> > > > > > 
> > > > > > gcc/cp/ChangeLog:
> > > > > > 
> > > > > > 	PR c++/96675
> > > > > > 	PR c++/96742
> > > > > > 	* pt.c (tsubst_copy_and_build): Call uses_template_parms instead of
> > > > > > 	type_dependent_expression_p_push.  Only call uses_template_parms
> > > > > > 	for expressions that are potential_constant_expression.
> > > > > > 
> > > > > > gcc/testsuite/ChangeLog:
> > > > > > 
> > > > > > 	PR c++/96675
> > > > > > 	PR c++/96742
> > > > > > 	* g++.dg/warn/Wdiv-by-zero-3.C: Turn dg-warning into dg-bogus.
> > > > > > 	* g++.dg/warn/Wtautological-compare3.C: New test.
> > > > > > 	* g++.dg/warn/Wtype-limits5.C: New test.
> > > > > > 	* g++.old-deja/g++.pt/crash10.C: Remove dg-warning.
> > > > > > ---
> > > > > >     gcc/cp/pt.c                                        |  6 ++++--
> > > > > >     gcc/testsuite/g++.dg/warn/Wdiv-by-zero-3.C         |  6 ++++--
> > > > > >     gcc/testsuite/g++.dg/warn/Wtautological-compare3.C | 11 +++++++++++
> > > > > >     gcc/testsuite/g++.dg/warn/Wtype-limits5.C          | 11 +++++++++++
> > > > > >     gcc/testsuite/g++.old-deja/g++.pt/crash10.C        |  1 -
> > > > > >     5 files changed, 30 insertions(+), 5 deletions(-)
> > > > > >     create mode 100644 gcc/testsuite/g++.dg/warn/Wtautological-compare3.C
> > > > > >     create mode 100644 gcc/testsuite/g++.dg/warn/Wtype-limits5.C
> > > > > > 
> > > > > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > > > > > index dc664ec3798..8aa0bc2c0d8 100644
> > > > > > --- a/gcc/cp/pt.c
> > > > > > +++ b/gcc/cp/pt.c
> > > > > > @@ -19618,8 +19618,10 @@ tsubst_copy_and_build (tree t,
> > > > > >           {
> > > > > >     	/* If T was type-dependent, suppress warnings that depend on the range
> > > > > >     	   of the types involved.  */
> > > > > > -	bool was_dep = type_dependent_expression_p_push (t);
> > > > > > -
> > > > > > +	++processing_template_decl;
> > > > > > +	const bool was_dep = (!potential_constant_expression (t)
> > > > > > +			      || uses_template_parms (t));
> > > > > 
> > > > > We don't want to suppress warnings for a non-constant expression that uses
> > > > > no template parms.  So maybe
> > > > 
> > > > Fair enough.
> > > > 
> > > > > potential_c_e ? value_d : type_d
> > > > 
> > > > That works for all the cases I have.
> > > > 
> > > > > ?  Or perhaps instantiation_dependent_expression_p.
> > > > 
> > > > i_d_e_p would still crash in C++98 :(.
> > > 
> > > Perhaps we should protect the value_d call in i_d_e_p with potential_c_e?
> > 
> > Yeah, probably.  But then we should also guard the call to value_d in
> > uses_template_parms.  I can apply such a patch if it tests fine, if you
> > want.
> 
> Or change uses_template_parms to use i_d.

Experimenting with this revealed a curious issue: when we have
__PRETTY_FUNCTION__ in a template function, we set its DECL_VALUE_EXPR
to error_mark_node (cp_make_fname_decl), so potential_c_e returns false
when it gets it, but value_dependent_expression_p handles it specially
and says true.  So this patch

--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -27277,7 +27277,8 @@ bool
 instantiation_dependent_expression_p (tree expression)
 {
   return (instantiation_dependent_uneval_expression_p (expression)
-     || value_dependent_expression_p (expression));
+     || (potential_constant_expression (expression)
+         && value_dependent_expression_p (expression)));
 }
 
 /* Like type_dependent_expression_p, but it also works while not processing

breaks lambda-generic-pretty1.C.  ISTM that potential_c_e should return
true for a DECL_PRETTY_FUNCTION_P with error DECL_VALUE_EXPR.

Marek