C++ PATCH for c++/85847, ICE with template_id_expr in new()

[Marek Polacek]

On Wed, May 23, 2018 at 12:45:11PM -0400, Jason Merrill wrote:
> On Wed, May 23, 2018 at 9:46 AM, Marek Polacek <polacek@redhat.com> wrote:
> > The diagnostic code in build_new{,_1} was using maybe_constant_value to fold
> > the array length, but that breaks while parsing a template, because we might
> > then leak template codes to the constexpr machinery.
> >
> > Bootstrapped/regtested on x86_64-linux, ok for trunk/8?
> >
> > 2018-05-23  Marek Polacek  <polacek@redhat.com>
> >
> >         PR c++/85847
> >         * init.c (build_new_1): Use fold_non_dependent_expr.
> >         (build_new): Likewise.
> >
> >         * g++.dg/cpp0x/new3.C: New test.
> >
> > @@ -2860,7 +2860,7 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
> >    /* Lots of logic below. depends on whether we have a constant number of
> >       elements, so go ahead and fold it now.  */
> >    if (outer_nelts)
> > -    outer_nelts = maybe_constant_value (outer_nelts);
> > +    outer_nelts = fold_non_dependent_expr (outer_nelts);
> 
> If outer_nelts is non-constant, this will mean that it ends up
> instantiated but still non-constant, which can lead to problems when
> the result is used in building up other expressions.
> 
> I think we want to put the result of folding in a separate variable
> for use with things that want to know about a constant size, and keep
> the original outer_nelts for use in building outer_nelts_check.
> 
> >        /* Try to determine the constant value only for the purposes
> >          of the diagnostic below but continue to use the original
> >          value and handle const folding later.  */
> > -      const_tree cst_nelts = maybe_constant_value (nelts);
> > +      const_tree cst_nelts = fold_non_dependent_expr (nelts);
> 
> ...like we do here.

Like this?

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2018-05-23  Marek Polacek  <polacek@redhat.com>

	PR c++/85847
	* init.c (build_new_1): Use fold_non_dependent_expr.  Use a dedicated
	variable for its result.  Fix a condition.
	(build_new): Use fold_non_dependent_expr.  Tweak a condition.

	* g++.dg/cpp0x/new3.C: New test.

diff --git gcc/cp/init.c gcc/cp/init.c
index b558742abf6..cd0110a1e19 100644
--- gcc/cp/init.c
+++ gcc/cp/init.c
@@ -2857,10 +2857,9 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
       outer_nelts_from_type = true;
     }
 
-  /* Lots of logic below. depends on whether we have a constant number of
+  /* Lots of logic below depends on whether we have a constant number of
      elements, so go ahead and fold it now.  */
-  if (outer_nelts)
-    outer_nelts = maybe_constant_value (outer_nelts);
+  const_tree cst_outer_nelts = fold_non_dependent_expr (outer_nelts);
 
   /* If our base type is an array, then make sure we know how many elements
      it has.  */
@@ -2912,11 +2911,12 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
   /* Warn if we performed the (T[N]) to T[N] transformation and N is
      variable.  */
   if (outer_nelts_from_type
-      && !TREE_CONSTANT (outer_nelts))
+      && cst_outer_nelts != NULL_TREE
+      && !TREE_CONSTANT (cst_outer_nelts))
     {
       if (complain & tf_warning_or_error)
 	{
-	  pedwarn (EXPR_LOC_OR_LOC (outer_nelts, input_location), OPT_Wvla,
+	  pedwarn (EXPR_LOC_OR_LOC (cst_outer_nelts, input_location), OPT_Wvla,
 		   typedef_variant_p (orig_type)
 		   ? G_("non-constant array new length must be specified "
 			"directly, not by typedef")
@@ -3011,9 +3011,10 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
 
       size = size_binop (MULT_EXPR, size, fold_convert (sizetype, nelts));
 
-      if (INTEGER_CST == TREE_CODE (outer_nelts))
+      if (cst_outer_nelts != NULL_TREE
+	  && TREE_CODE (cst_outer_nelts) == INTEGER_CST)
 	{
-	  if (tree_int_cst_lt (max_outer_nelts_tree, outer_nelts))
+	  if (tree_int_cst_lt (max_outer_nelts_tree, cst_outer_nelts))
 	    {
 	      /* When the array size is constant, check it at compile time
 		 to make sure it doesn't exceed the implementation-defined
@@ -3639,13 +3640,13 @@ build_new (vec<tree, va_gc> **placement, tree type, tree nelts,
       /* Try to determine the constant value only for the purposes
 	 of the diagnostic below but continue to use the original
 	 value and handle const folding later.  */
-      const_tree cst_nelts = maybe_constant_value (nelts);
+      const_tree cst_nelts = fold_non_dependent_expr (nelts);
 
       /* The expression in a noptr-new-declarator is erroneous if it's of
 	 non-class type and its value before converting to std::size_t is
 	 less than zero. ... If the expression is a constant expression,
 	 the program is ill-fomed.  */
-      if (INTEGER_CST == TREE_CODE (cst_nelts)
+      if (TREE_CODE (cst_nelts) == INTEGER_CST
 	  && tree_int_cst_sgn (cst_nelts) == -1)
 	{
 	  if (complain & tf_error)
diff --git gcc/testsuite/g++.dg/cpp0x/new3.C gcc/testsuite/g++.dg/cpp0x/new3.C
index e69de29bb2d..c388acf552e 100644
--- gcc/testsuite/g++.dg/cpp0x/new3.C
+++ gcc/testsuite/g++.dg/cpp0x/new3.C
@@ -0,0 +1,11 @@
+// PR c++/85847
+// { dg-do compile { target c++11 } }
+
+template <class>
+int f(int b) { return b; }
+
+template <class>
+void g()
+{
+  auto a = new int[f<int>(2), 2];
+}