[patch] Don't let invalid methods mark their RecordDecl invalid

[Nico Weber]

Hi,

if a method decl is invalid, clang currently sometimes marks the method's
record decl invalid, and sometimes doesn't (and this ends up confusing
record layout). For consistency, and to fix PR18284, never do this.

(The code to do this was added in
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20100809/033154.html,
but the test from that revision is still passing.)

See also this thread
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2013-December/034216.html for
some discussion.

Nico
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131220/ac94f338/attachment.html>
-------------- next part --------------
Index: test/SemaCXX/pr18284-crash-on-invalid.cpp
===================================================================
--- test/SemaCXX/pr18284-crash-on-invalid.cpp	(revision 0)
+++ test/SemaCXX/pr18284-crash-on-invalid.cpp	(revision 0)
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// Don't crash (PR18284).
+
+namespace n1 {
+class A { };
+class C { A a; };
+
+A::RunTest() {} // expected-error {{C++ requires a type specifier for all declarations}}
+
+void f() {
+  new C;
+}
+} // namespace n1
+
+namespace n2 {
+class A { };
+class C : public A { };
+
+A::RunTest() {} // expected-error {{C++ requires a type specifier for all declarations}}
+
+void f() {
+  new C;
+}
+} // namespace n1
Index: test/SemaCXX/constructor-initializer.cpp
===================================================================
--- test/SemaCXX/constructor-initializer.cpp	(revision 197844)
+++ test/SemaCXX/constructor-initializer.cpp	(working copy)
@@ -232,15 +232,14 @@
 // <rdar://problem/8308215>: don't crash.
 // Lots of questionable recovery here;  errors can change.
 namespace test3 {
-  class A : public std::exception {}; // expected-error {{undeclared identifier}} expected-error {{expected class name}} expected-note 4 {{candidate}}
+  class A : public std::exception {}; // expected-error {{undeclared identifier}} expected-error {{expected class name}} expected-note 2 {{candidate}}
   class B : public A {
   public:
     B(const String& s, int e=0) // expected-error {{unknown type name}} 
       : A(e), m_String(s) , m_ErrorStr(__null) {} // expected-error {{no matching constructor}} expected-error {{does not name}}
     B(const B& e)
       : A(e), m_String(e.m_String), m_ErrorStr(__null) { // expected-error {{does not name}} \
-      // expected-error {{no member named 'm_String' in 'test3::B'}} \
-      // expected-error {{no matching}}
+      // expected-error {{no member named 'm_String' in 'test3::B'}}
     }
   };
 }
Index: test/SemaCXX/pr13394-crash-on-invalid.cpp
===================================================================
--- test/SemaCXX/pr13394-crash-on-invalid.cpp	(revision 197844)
+++ test/SemaCXX/pr13394-crash-on-invalid.cpp	(working copy)
@@ -8,12 +8,12 @@
 }
 namespace gatekeeper_v1 {
   namespace gatekeeper_factory_v1 {
-    struct closure_t { // expected-note {{'closure_t' declared here}}
+    struct closure_t { // expected-note {{'closure_t' declared here}} expected-note {{'gatekeeper_factory_v1::closure_t' declared here}}
       gatekeeper_v1::closure_t* create(); // expected-error {{no type named 'closure_t' in namespace 'gatekeeper_v1'; did you mean simply 'closure_t'?}}
     };
   }
   // FIXME: Typo correction should remove the 'gatekeeper_v1::' name specifier
-  gatekeeper_v1::closure_t *x; // expected-error-re {{no type named 'closure_t' in namespace 'gatekeeper_v1'{{$}}}}
+  gatekeeper_v1::closure_t *x; // expected-error {{no type named 'closure_t' in namespace 'gatekeeper_v1'; did you mean 'gatekeeper_factory_v1::closure_t'}}
 }
 
 namespace Foo {
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp	(revision 197844)
+++ lib/Sema/SemaDecl.cpp	(working copy)
@@ -7176,12 +7176,7 @@
       if (!NewFD->isInvalidDecl() && NewFD->isMSVCRTEntryPoint())
         CheckMSVCRTEntryPoint(NewFD);
 
-      if (NewFD->isInvalidDecl()) {
-        // If this is a class member, mark the class invalid immediately.
-        // This avoids some consistency errors later.
-        if (CXXMethodDecl* methodDecl = dyn_cast<CXXMethodDecl>(NewFD))
-          methodDecl->getParent()->setInvalidDecl();
-      } else
+      if (!NewFD->isInvalidDecl())
         D.setRedeclaration(CheckFunctionDeclaration(S, NewFD, Previous,
                                                     isExplicitSpecialization));
     }