diff mbox

[C++] PR 70202 ("ICE on invalid code in build_simple_base_path, at cp/class.c:579")

Message ID 5751429F.4070309@oracle.com
State New
Headers show

Commit Message

Paolo Carlini June 3, 2016, 8:41 a.m. UTC 
Hi,

in this error recovery issue, after a sensible error message about 
duplicate base type and a redundant one about an incomplete type (the 
type with the erroneous base type) we finally crash much later in 
build_simple_base_path. Yesterday I noticed that elsewhere we don't 
check the return value of xref_basetypes (which only returns false after 
an hard error) and error recovery seems better if we don't zero the type 
in such cases (we don't end up with this weird half-erroneous, 
half-incomplete type declaration which we have to tell from the good 
ones downstream, we have instead an usable reference for the informs 
about duplicated definitions (see yesterday's patch), etc.). I also 
found the commit which introduced the zeroing, r189582, back in 2012, 
and the testcase which was crashing back then is now correctly handled 
anyway. Thus, all in all, I'm proposing the below for trunk. I guess we 
can quickly back it out if we realize that error recovery gets much 
worse in other as yet unknown situations.

Tested x86_64-linux.

Thanks,
Paolo.

/////////////////////////////
/cp
2016-06-03  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/70202
	* parser.c (cp_parser_class_head): When xref_basetypes fails and
	emits an error do not zero the type.

/testsuite
2016-06-03  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/70202
	* g++.dg/inherit/crash5.C: New.
	* g++.dg/inherit/virtual1.C: Adjust.

Comments

Jason Merrill June 3, 2016, 7:56 p.m. UTC | #1 
OK.

Jason
Paolo Carlini June 9, 2016, 11:23 a.m. UTC | #2 
.. unfortunately with the patchlet applied we regress on c++/71465. 
Thus I'm going to revert it for the time being with a testcase from the 
new bug.

Thanks,
Paolo.
diff mbox

Patch

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 237046)
+++ cp/parser.c	(working copy)
@@ -22050,9 +22050,8 @@  cp_parser_class_head (cp_parser* parser,
 
   /* If we're really defining a class, process the base classes.
      If they're invalid, fail.  */
-  if (type && cp_lexer_next_token_is (parser->lexer, CPP_OPEN_BRACE)
-      && !xref_basetypes (type, bases))
-    type = NULL_TREE;
+  if (type && cp_lexer_next_token_is (parser->lexer, CPP_OPEN_BRACE))
+    xref_basetypes (type, bases);
 
  done:
   /* Leave the scope given by the nested-name-specifier.  We will
Index: testsuite/g++.dg/inherit/crash5.C
===================================================================
--- testsuite/g++.dg/inherit/crash5.C	(revision 0)
+++ testsuite/g++.dg/inherit/crash5.C	(working copy)
@@ -0,0 +1,10 @@ 
+// PR c++/70202
+
+class A
+{
+  virtual void foo () { }
+};
+class B : public A, A { };  // { dg-error "duplicate base type" }
+
+B b1, &b2 = b1;
+A a = b2;
Index: testsuite/g++.dg/inherit/virtual1.C
===================================================================
--- testsuite/g++.dg/inherit/virtual1.C	(revision 237045)
+++ testsuite/g++.dg/inherit/virtual1.C	(working copy)
@@ -5,8 +5,8 @@  struct A
     virtual ~A() {}
 };
 
-struct B : A, virtual A {};     // { dg-error "duplicate base|forward declaration" }
+struct B : A, virtual A {};     // { dg-error "duplicate base" }
 
-struct C : A, B {};             // { dg-error "duplicate base|invalid use" }
+struct C : A, B {};             // { dg-error "duplicate base" }
 
-C c;                            // { dg-error "aggregate" }
+C c;