diff mbox series

[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) | expand

Commit Message

Jakub Jelinek March 22, 2018, 9:27 p.m. UTC 
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. UTC | #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. UTC | #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. UTC | #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. UTC | #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. UTC | #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
diff mbox series

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;
+}