[C++] Fix error-recovery in compute_array_index_type (PR c++/85015)

Message ID 20180322212759.GV8577@tucnak
State New
Headers show
Series
  • [C++] Fix error-recovery in compute_array_index_type (PR c++/85015)
Related show

Commit Message

Jakub Jelinek March 22, 2018, 9:27 p.m.
Hi!

We ICE during error-recovery on the following testcase, compute_array_index_type
checks size for error_operand_p early (and it is not error operand; it is
<indirect_ref <var_decl c>> where c is of reference type with
error_mark_node DECL_INITIAL), then calls mark_rvalue_use (which returns error_mark_node), but we let it go
through maybe_constant_size etc. and because error_mark_node is
!TREE_CONSTANT, set it back to the original size.  Unfortunatley later on
that results in ICE, because
9662		itype = cp_build_binary_op (input_location,
9663					    MINUS_EXPR,
9664					    cp_convert (ssizetype, size, complain),
9665					    cp_convert (ssizetype, integer_one_node,
9666							complain),
9667					    complain);
errors on it the second time:
error: use of local variable with automatic storage from containing function
(the first error was during mark_rvalue_use) and nothing gives up anymore,
so we build a domain with error_mark_node as maximum.
Fixed by giving up earlier if mark_rvalue_use fails.

Initially I had the second error_operand_p check before the
if (!TREE_CONSTANT (size)) check, but that unfortunately doesn't work,
it broke
FAIL: g++.dg/cpp0x/constexpr-47969.C  -std=c++11  (test for errors, line 11)
FAIL: g++.dg/cpp0x/constexpr-47969.C  -std=c++14  (test for errors, line 11)
FAIL: g++.dg/cpp0x/constexpr-ex2.C  -std=c++11  (test for errors, line 21)
FAIL: g++.dg/cpp0x/constexpr-ex2.C  -std=c++14  (test for errors, line 21)
FAIL: g++.dg/cpp0x/scoped_enum2.C  -std=c++11  (test for errors, line 7)
FAIL: g++.dg/cpp0x/scoped_enum2.C  -std=c++14  (test for errors, line 7)
FAIL: g++.dg/ext/stmtexpr15.C  -std=gnu++11  (test for errors, line 6)
FAIL: g++.dg/ext/stmtexpr15.C  -std=gnu++14  (test for errors, line 6)
FAIL: g++.dg/ext/stmtexpr15.C  -std=gnu++98  (test for errors, line 6)
FAIL: g++.dg/gomp/pr47963.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/gomp/pr47963.C  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/gomp/pr47963.C  -std=gnu++98 (test for excess errors)
where the errors weren't reported anymore.

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

2018-03-22  Jakub Jelinek  <jakub@redhat.com>

	PR c++/85015
	* decl.c (compute_array_index_type): Return error_mark_node if
	mark_rvalue_use or maybe_constant_value returns error_operand_p.

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


	Jakub

Comments

Jason Merrill March 23, 2018, 1:31 p.m. | #1
On Thu, Mar 22, 2018 at 5:27 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> We ICE during error-recovery on the following testcase, compute_array_index_type

> checks size for error_operand_p early (and it is not error operand; it is

> <indirect_ref <var_decl c>> where c is of reference type with

> error_mark_node DECL_INITIAL), then calls mark_rvalue_use (which returns error_mark_node), but we let it go

> through maybe_constant_size etc. and because error_mark_node is

> !TREE_CONSTANT, set it back to the original size.


Hmm, maybe we need to also update osize with the result of
mark_rvalue_use.  Or flip the logic of the constant value block to use
a local variable and only change size if the result is constant.

Jason
Jakub Jelinek March 23, 2018, 2:17 p.m. | #2
On Fri, Mar 23, 2018 at 09:31:53AM -0400, Jason Merrill wrote:
> On Thu, Mar 22, 2018 at 5:27 PM, Jakub Jelinek <jakub@redhat.com> wrote:

> > We ICE during error-recovery on the following testcase, compute_array_index_type

> > checks size for error_operand_p early (and it is not error operand; it is

> > <indirect_ref <var_decl c>> where c is of reference type with

> > error_mark_node DECL_INITIAL), then calls mark_rvalue_use (which returns error_mark_node), but we let it go

> > through maybe_constant_size etc. and because error_mark_node is

> > !TREE_CONSTANT, set it back to the original size.

> 

> Hmm, maybe we need to also update osize with the result of

> mark_rvalue_use.  Or flip the logic of the constant value block to use

> a local variable and only change size if the result is constant.


Like this?  Passes check-c++-all...

2018-03-23  Jakub Jelinek  <jakub@redhat.com>

	PR c++/85015
	* decl.c (compute_array_index_type): Return error_mark_node if
	mark_rvalue_use or maybe_constant_value returns error_operand_p.
	Set osize to mark_rvalue_use result.

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

--- gcc/cp/decl.c.jj	2018-03-22 22:23:10.120015621 +0100
+++ gcc/cp/decl.c	2018-03-23 14:52:07.274893996 +0100
@@ -9520,7 +9520,10 @@ compute_array_index_type (tree name, tre
 
   if (!type_dependent_expression_p (size))
     {
-      size = mark_rvalue_use (size);
+      osize = size = mark_rvalue_use (size);
+
+      if (error_operand_p (size))
+	return error_mark_node;
 
       if (cxx_dialect < cxx11 && TREE_CODE (size) == NOP_EXPR
 	  && TREE_SIDE_EFFECTS (size))
@@ -9534,11 +9537,10 @@ compute_array_index_type (tree name, tre
 
 	  if (!TREE_CONSTANT (size))
 	    size = osize;
+	  else if (error_operand_p (size))
+	    return error_mark_node;
 	}
 
-      if (error_operand_p (size))
-	return error_mark_node;
-
       /* The array bound must be an integer type.  */
       tree type = TREE_TYPE (size);
       if (!INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P (type))
--- gcc/testsuite/g++.dg/cpp0x/pr85015.C.jj	2018-03-23 14:51:19.207892516 +0100
+++ gcc/testsuite/g++.dg/cpp0x/pr85015.C	2018-03-23 14:51:19.207892516 +0100
@@ -0,0 +1,12 @@
+// PR c++/85015
+// { dg-do compile { target c++11 } }
+// { dg-options "" }
+
+void
+foo ()
+{
+  int &&c = v + 1;		// { dg-error "was not declared in this scope" }
+  struct S {			// { dg-message "declared here" "" { target *-*-* } .-1 }
+    void bar () { int a[c]; }	// { dg-error "use of local variable with automatic storage from containing function" }
+  } e;
+}


	Jakub
Jason Merrill March 23, 2018, 4:32 p.m. | #3
On Fri, Mar 23, 2018 at 10:17 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Mar 23, 2018 at 09:31:53AM -0400, Jason Merrill wrote:

>> On Thu, Mar 22, 2018 at 5:27 PM, Jakub Jelinek <jakub@redhat.com> wrote:

>> > We ICE during error-recovery on the following testcase, compute_array_index_type

>> > checks size for error_operand_p early (and it is not error operand; it is

>> > <indirect_ref <var_decl c>> where c is of reference type with

>> > error_mark_node DECL_INITIAL), then calls mark_rvalue_use (which returns error_mark_node), but we let it go

>> > through maybe_constant_size etc. and because error_mark_node is

>> > !TREE_CONSTANT, set it back to the original size.

>>

>> Hmm, maybe we need to also update osize with the result of

>> mark_rvalue_use.  Or flip the logic of the constant value block to use

>> a local variable and only change size if the result is constant.

>

> Like this?  Passes check-c++-all...

>

> 2018-03-23  Jakub Jelinek  <jakub@redhat.com>

>

>         PR c++/85015

>         * decl.c (compute_array_index_type): Return error_mark_node if

>         mark_rvalue_use or maybe_constant_value returns error_operand_p.

>         Set osize to mark_rvalue_use result.

>

>         * g++.dg/cpp0x/pr85015.C: New test.

>

> --- gcc/cp/decl.c.jj    2018-03-22 22:23:10.120015621 +0100

> +++ gcc/cp/decl.c       2018-03-23 14:52:07.274893996 +0100

> @@ -9520,7 +9520,10 @@ compute_array_index_type (tree name, tre

>

>    if (!type_dependent_expression_p (size))

>      {

> -      size = mark_rvalue_use (size);

> +      osize = size = mark_rvalue_use (size);


With this, I think we shouldn't need the other changes.

Jason
Jakub Jelinek March 23, 2018, 5:26 p.m. | #4
On Fri, Mar 23, 2018 at 12:32:20PM -0400, Jason Merrill wrote:
> > 2018-03-23  Jakub Jelinek  <jakub@redhat.com>

> >

> >         PR c++/85015

> >         * decl.c (compute_array_index_type): Return error_mark_node if

> >         mark_rvalue_use or maybe_constant_value returns error_operand_p.

> >         Set osize to mark_rvalue_use result.

> >

> >         * g++.dg/cpp0x/pr85015.C: New test.

> >

> > --- gcc/cp/decl.c.jj    2018-03-22 22:23:10.120015621 +0100

> > +++ gcc/cp/decl.c       2018-03-23 14:52:07.274893996 +0100

> > @@ -9520,7 +9520,10 @@ compute_array_index_type (tree name, tre

> >

> >    if (!type_dependent_expression_p (size))

> >      {

> > -      size = mark_rvalue_use (size);

> > +      osize = size = mark_rvalue_use (size);

> 

> With this, I think we shouldn't need the other changes.


Yes, at least check-c++-all doesn't regress with this, will do now full
bootstrap/regtest.  Ok if it passes?

2018-03-23  Jakub Jelinek  <jakub@redhat.com>

	PR c++/85015
	* decl.c (compute_array_index_type): Set osize to mark_rvalue_use
	result.

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

--- gcc/cp/decl.c.jj	2018-03-22 22:23:10.120015621 +0100
+++ gcc/cp/decl.c	2018-03-23 17:33:52.412071232 +0100
@@ -9520,7 +9520,7 @@ compute_array_index_type (tree name, tre
 
   if (!type_dependent_expression_p (size))
     {
-      size = mark_rvalue_use (size);
+      osize = size = mark_rvalue_use (size);
 
       if (cxx_dialect < cxx11 && TREE_CODE (size) == NOP_EXPR
 	  && TREE_SIDE_EFFECTS (size))
--- gcc/testsuite/g++.dg/cpp0x/pr85015.C.jj	2018-03-23 14:51:19.207892516 +0100
+++ gcc/testsuite/g++.dg/cpp0x/pr85015.C	2018-03-23 14:51:19.207892516 +0100
@@ -0,0 +1,12 @@
+// PR c++/85015
+// { dg-do compile { target c++11 } }
+// { dg-options "" }
+
+void
+foo ()
+{
+  int &&c = v + 1;		// { dg-error "was not declared in this scope" }
+  struct S {			// { dg-message "declared here" "" { target *-*-* } .-1 }
+    void bar () { int a[c]; }	// { dg-error "use of local variable with automatic storage from containing function" }
+  } e;
+}


	Jakub
Jason Merrill March 23, 2018, 6:20 p.m. | #5
OK.

On Fri, Mar 23, 2018 at 1:26 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Mar 23, 2018 at 12:32:20PM -0400, Jason Merrill wrote:

>> > 2018-03-23  Jakub Jelinek  <jakub@redhat.com>

>> >

>> >         PR c++/85015

>> >         * decl.c (compute_array_index_type): Return error_mark_node if

>> >         mark_rvalue_use or maybe_constant_value returns error_operand_p.

>> >         Set osize to mark_rvalue_use result.

>> >

>> >         * g++.dg/cpp0x/pr85015.C: New test.

>> >

>> > --- gcc/cp/decl.c.jj    2018-03-22 22:23:10.120015621 +0100

>> > +++ gcc/cp/decl.c       2018-03-23 14:52:07.274893996 +0100

>> > @@ -9520,7 +9520,10 @@ compute_array_index_type (tree name, tre

>> >

>> >    if (!type_dependent_expression_p (size))

>> >      {

>> > -      size = mark_rvalue_use (size);

>> > +      osize = size = mark_rvalue_use (size);

>>

>> With this, I think we shouldn't need the other changes.

>

> Yes, at least check-c++-all doesn't regress with this, will do now full

> bootstrap/regtest.  Ok if it passes?

>

> 2018-03-23  Jakub Jelinek  <jakub@redhat.com>

>

>         PR c++/85015

>         * decl.c (compute_array_index_type): Set osize to mark_rvalue_use

>         result.

>

>         * g++.dg/cpp0x/pr85015.C: New test.

>

> --- gcc/cp/decl.c.jj    2018-03-22 22:23:10.120015621 +0100

> +++ gcc/cp/decl.c       2018-03-23 17:33:52.412071232 +0100

> @@ -9520,7 +9520,7 @@ compute_array_index_type (tree name, tre

>

>    if (!type_dependent_expression_p (size))

>      {

> -      size = mark_rvalue_use (size);

> +      osize = size = mark_rvalue_use (size);

>

>        if (cxx_dialect < cxx11 && TREE_CODE (size) == NOP_EXPR

>           && TREE_SIDE_EFFECTS (size))

> --- gcc/testsuite/g++.dg/cpp0x/pr85015.C.jj     2018-03-23 14:51:19.207892516 +0100

> +++ gcc/testsuite/g++.dg/cpp0x/pr85015.C        2018-03-23 14:51:19.207892516 +0100

> @@ -0,0 +1,12 @@

> +// PR c++/85015

> +// { dg-do compile { target c++11 } }

> +// { dg-options "" }

> +

> +void

> +foo ()

> +{

> +  int &&c = v + 1;             // { dg-error "was not declared in this scope" }

> +  struct S {                   // { dg-message "declared here" "" { target *-*-* } .-1 }

> +    void bar () { int a[c]; }  // { dg-error "use of local variable with automatic storage from containing function" }

> +  } e;

> +}

>

>

>         Jakub

Patch

--- gcc/cp/decl.c.jj	2018-03-21 22:47:16.642996454 +0100
+++ gcc/cp/decl.c	2018-03-22 19:37:10.039385428 +0100
@@ -9522,6 +9522,9 @@  compute_array_index_type (tree name, tre
     {
       size = mark_rvalue_use (size);
 
+      if (error_operand_p (size))
+	return error_mark_node;
+
       if (cxx_dialect < cxx11 && TREE_CODE (size) == NOP_EXPR
 	  && TREE_SIDE_EFFECTS (size))
 	/* In C++98, we mark a non-constant array bound with a magic
@@ -9534,11 +9537,10 @@  compute_array_index_type (tree name, tre
 
 	  if (!TREE_CONSTANT (size))
 	    size = osize;
+	  else if (error_operand_p (size))
+	    return error_mark_node;
 	}
 
-      if (error_operand_p (size))
-	return error_mark_node;
-
       /* The array bound must be an integer type.  */
       tree type = TREE_TYPE (size);
       if (!INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P (type))
--- gcc/testsuite/g++.dg/cpp0x/pr85015.C.jj	2018-03-22 19:27:12.856154008 +0100
+++ gcc/testsuite/g++.dg/cpp0x/pr85015.C	2018-03-22 19:27:12.856154008 +0100
@@ -0,0 +1,12 @@ 
+// PR c++/85015
+// { dg-do compile { target c++11 } }
+// { dg-options "" }
+
+void
+foo ()
+{
+  int &&c = v + 1;		// { dg-error "was not declared in this scope" }
+  struct S {			// { dg-message "declared here" "" { target *-*-* } .-1 }
+    void bar () { int a[c]; }	// { dg-error "use of local variable with automatic storage from containing function" }
+  } e;
+}