Improve our semantic error recovery.

When an error made a record member invalid, the record would stay as "isBeingDefined" and
not "completeDefinition". Even easily recoverable errors ended up propagating records in
such "beingDefined" state, for example:

struct A {
  ~A() const; // expected-error {{'const' qualifier is not allowed on a destructor}}
};
struct B : A {}; // A & B would stay as "not complete definition" and "being defined".

This weird state was impending lookups in the records and hitting assertion in the ASTWriter.

Part of rdar://11007039

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@152432 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 47 insertions, 15 deletions

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 6c25a20e24..5febad616c 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -2378,8 +2378,11 @@ Decl *Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS,
       Tag = CTD->getTemplatedDecl();
   }
 
-  if (Tag)
+  if (Tag) {
     Tag->setFreeStanding();
+    if (Tag->isInvalidDecl())
+      return Tag;
+  }
 
   if (unsigned TypeQuals = DS.getTypeQualifiers()) {
     // Enforce C99 6.7.3p2: "Types other than pointer types derived from object
@@ -8601,6 +8604,13 @@ void Sema::ActOnTagFinishDefinition(Scope *S, Decl *TagD,
   TagDecl *Tag = cast<TagDecl>(TagD);
   Tag->setRBraceLoc(RBraceLoc);
 
+  // Make sure we "complete" the definition even it is invalid.
+  if (Tag->isBeingDefined()) {
+    assert(Tag->isInvalidDecl() && "We should already have completed it");
+    if (RecordDecl *RD = dyn_cast<RecordDecl>(Tag))
+      RD->completeDefinition();
+  }
+
   if (isa<CXXRecordDecl>(Tag))
     FieldCollector->FinishClass();
 
@@ -8632,6 +8642,12 @@ void Sema::ActOnTagDefinitionError(Scope *S, Decl *TagD) {
   TagDecl *Tag = cast<TagDecl>(TagD);
   Tag->setInvalidDecl();
 
+  // Make sure we "complete" the definition even it is invalid.
+  if (Tag->isBeingDefined()) {
+    if (RecordDecl *RD = dyn_cast<RecordDecl>(Tag))
+      RD->completeDefinition();
+  }
+
   // We're undoing ActOnTagStartDefinition here, not
   // ActOnStartCXXMemberDeclarations, so we don't have to mess with
   // the FieldCollector.
@@ -8840,11 +8856,19 @@ FieldDecl *Sema::CheckFieldDecl(DeclarationName Name, QualType T,
   }
 
   QualType EltTy = Context.getBaseElementType(T);
-  if (!EltTy->isDependentType() &&
-      RequireCompleteType(Loc, EltTy, diag::err_field_incomplete)) {
-    // Fields of incomplete type force their record to be invalid.
-    Record->setInvalidDecl();
-    InvalidDecl = true;
+  if (!EltTy->isDependentType()) {
+    if (RequireCompleteType(Loc, EltTy, diag::err_field_incomplete)) {
+      // Fields of incomplete type force their record to be invalid.
+      Record->setInvalidDecl();
+      InvalidDecl = true;
+    } else {
+      NamedDecl *Def;
+      EltTy->isIncompleteType(&Def);
+      if (Def && Def->isInvalidDecl()) {
+        Record->setInvalidDecl();
+        InvalidDecl = true;
+      }
+    }
   }
 
   // C99 6.7.2.1p8: A member of a structure or union may have any type other
diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp
index 8e950ac207..ea92b9dcd5 100644
--- a/lib/Sema/SemaLookup.cpp
+++ b/lib/Sema/SemaLookup.cpp
@@ -527,10 +527,6 @@ static bool LookupBuiltin(Sema &S, LookupResult &R) {
 /// the class at this point.
 static bool CanDeclareSpecialMemberFunction(ASTContext &Context,
                                             const CXXRecordDecl *Class) {
-  // Don't do it if the class is invalid.
-  if (Class->isInvalidDecl())
-    return false;
-
   // We need to have a definition for the class.
   if (!Class->getDefinition() || Class->isDependentContext())
     return false;
diff --git a/test/Index/pch-with-errors.c b/test/Index/pch-with-errors.c
index d35200041c..691eab4585 100644
--- a/test/Index/pch-with-errors.c
+++ b/test/Index/pch-with-errors.c
@@ -5,6 +5,18 @@
 void erroneous(int);
 void erroneous(float);
 
+struct bar;
+struct zed {
+  bar g;
+};
+struct baz {
+  zed h;
+};
+
+struct S {
+  {
+;
+
 #else
 
 void foo(void) {
@@ -17,8 +29,8 @@ void foo(void) {
 // RUN: c-index-test -test-load-source local %s -include %t.h -Xclang -detailed-preprocessing-record | FileCheck -check-prefix=CHECK-PARSE %s
 // RUN: c-index-test -index-file %s -include %t.h -Xclang -detailed-preprocessing-record | FileCheck -check-prefix=CHECK-INDEX %s
 
-// CHECK-PARSE: pch-with-errors.c:10:6: FunctionDecl=foo:10:6 (Definition) Extent=[10:1 - 12:2]
-// CHECK-PARSE: pch-with-errors.c:11:3: CallExpr=erroneous:5:6 Extent=[11:3 - 11:15]
+// CHECK-PARSE: pch-with-errors.c:{{.*}}:6: FunctionDecl=foo
+// CHECK-PARSE: pch-with-errors.c:{{.*}}:3: CallExpr=erroneous
 
 // CHECK-INDEX: [indexDeclaration]: kind: function | name: foo
 // CHECK-INDEX: [indexEntityReference]: kind: function | name: erroneous
diff --git a/test/SemaCXX/PR9460.cpp b/test/SemaCXX/PR9460.cpp
index 2cc435e495..0dd8446770 100644
--- a/test/SemaCXX/PR9460.cpp
+++ b/test/SemaCXX/PR9460.cpp
@@ -8,11 +8,11 @@ struct basic_string{
   basic_string(aT*);
 };
 
-struct runtime_error{ // expected-note {{candidate constructor}}
-  runtime_error( // expected-note {{candidate constructor}}
+struct runtime_error{
+  runtime_error(
 basic_string<char> struct{ // expected-error {{cannot combine with previous 'type-name' declaration specifier}}
 a(){ // expected-error {{requires a type specifier}}
-  runtime_error(0); // expected-error {{no matching conversion}}
+  runtime_error(0);
 }
 }
 );