This patch fixes pr11109 <http://llvm.org/bugs/show_bug.cgi?id=11109>, a
crash-on-invalid with the following source:

class foo { public

Matthias who reported the bug was nice enough to provide a basic fix (though
only attached it to the bug rather than emailing cfe-commits, so it went
unnoticed) but I've fixed up a few more things around here:

The original code was trying to consume the token following the access
specifier, this is what caused the crash in the above case*. I assume the
reason it was doing this was to catch the common case where a semicolon is
written instead of a colon. So to address this I special cased that - FixIt
replacing the semicolon with a colon (see class E in the attached test). But
in the general case, consuming this token just lead to bad error recovery
(see the comments for class D in the attached test) - "class foo { public
int i; };" would fail once for the missing colon, then again for the missing
type specifier for the declaration of 'i' because the 'int' had been
erroneously consumed while looking for a colon).

Please let me know if this looks good and I'll check it in and resolve the
bug,

- David

* since the token was eof - consuming when you're already at the end of the
real top level file causes the preprocessor to call a function on a null
pointer on Preprocessor.h:582. Perhaps we could add some asserts to check
that the current lexer (whichever one it is) is non-null when trying to lex.
I tried adding an assert in Parser::ConsumeToken to test that the token
being consumed was not eof, but that wasn't viable - apparently clang has
some cases where it actually lexes magic eofs. (the last eof of the outer
file cannot be consumed, as this test shows, and the inner eofs of any
included files never make it out of Lex)
Index: test/Parser/cxx-class.cpp
===================================================================
--- test/Parser/cxx-class.cpp	(revision 141846)
+++ test/Parser/cxx-class.cpp	(working copy)
@@ -40,3 +40,20 @@
   } y;
 } bug3177;
 
+// check that we don't consume the token after the access specifier 
+// when it's not a colon
+class D {
+public // expected-error{{expected ':'}}
+  int i;
+};
+
+// consume the token after the access specifier if it's a semicolon 
+// that was meant to be a colon
+class E {
+public; // expected-error{{expected ':'}}
+  int i;
+};
+
+// PR11109 must appear at the end of the source file
+class pr11109r3 { // expected-note{{to match this '{'}}
+  public // expected-error{{expected ':'}} expected-error{{expected '}'}} expected-error{{expected ';' after class}}
Index: lib/Parse/ParseDeclCXX.cpp
===================================================================
--- lib/Parse/ParseDeclCXX.cpp	(revision 141846)
+++ lib/Parse/ParseDeclCXX.cpp	(working copy)
@@ -2135,12 +2135,23 @@
         // Current token is a C++ access specifier.
         CurAS = AS;
         SourceLocation ASLoc = Tok.getLocation();
+        unsigned TokLength = Tok.getLength();
         ConsumeToken();
-        if (Tok.is(tok::colon))
-          Actions.ActOnAccessSpecifier(AS, ASLoc, Tok.getLocation());
-        else
-          Diag(Tok, diag::err_expected_colon);
-        ConsumeToken();
+        SourceLocation EndLoc;
+        if (Tok.is(tok::colon)) {
+          EndLoc = Tok.getLocation();
+          ConsumeToken();
+        } else if (Tok.is(tok::semi)) {
+          EndLoc = Tok.getLocation();
+          ConsumeToken();
+          Diag(EndLoc, diag::err_expected_colon) 
+            << FixItHint::CreateReplacement(EndLoc, ":");
+        } else {
+          EndLoc = ASLoc.getLocWithOffset(TokLength);
+          Diag(EndLoc, diag::err_expected_colon) 
+            << FixItHint::CreateInsertion(EndLoc, ":");
+        }
+        Actions.ActOnAccessSpecifier(AS, ASLoc, EndLoc);
         continue;
       }
 
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to