[PATCH] Improve error recovery around colon.

Richard Smith richard at metafoo.co.uk
Wed Apr 2 18:50:20 PDT 2014


  Sorry for the delay!


================
Comment at: include/clang/Basic/DiagnosticParseKinds.td:342-343
@@ -341,1 +341,4 @@
   "unexpected ':' in nested name specifier; did you mean '::'?">;
+def err_unexected_token_in_nested_name_spec : Error<
+  "unexpected '%0' in nested name specifier; did you mean ':'?">;
+def err_nested_name_spec_is_not_class : Error<
----------------
Either the second thing here should be `%1` (and you pass in `tok::colon`), or this diagnostic should have a better name. `"did you mean ':'?"` is not an obvious thing to say about a nested name specifier =)

Also, using `%1` will let you merge this with the previous diagnostic.

While you're here, please change `unexected` to `unexpected` in both diagnostic names. :)

================
Comment at: lib/Sema/SemaCXXScopeSpec.cpp:526
@@ +525,3 @@
+              << &Identifier;
+        if (IdDeclPtr)
+          *IdDeclPtr = ND;
----------------
This should probably be an `else`. If the caller picks `DontReportWrongMember`, `IdDeclPtr` is mandatory as they otherwise wouldn't know whether they're required to emit a diagnostic.

================
Comment at: lib/Sema/SemaCXXScopeSpec.cpp:521
@@ +520,3 @@
+            << &Identifier << getLangOpts().CPlusPlus;
+      if (NamedDecl *ND = R.getAsSingle<NamedDecl>()) {
+        if (RecoveryMode == ReportAll)
----------------
This will silently ignore the error if you're in `DontReportWrongMember` mode and name lookup finds something other than a single result.

================
Comment at: lib/Parse/ParseExprCXX.cpp:470-483
@@ -452,1 +469,16 @@
+                                              ObjectType, EnteringContext, SS,
+                                              ErrMode, &IdDecl)) {
+        // Identifier is not recognized as a nested name, but we can have
+        // mistyped '::' instead of ':'.
+        if (ColonIsSacred && IdDecl) {
+          Diag(CCLoc, diag::err_nested_name_spec_is_not_class)
+              << &II << getLangOpts().CPlusPlus
+              << FixItHint::CreateReplacement(CCLoc, ":");
+          Diag(IdDecl->getLocation(), diag::note_declared_at);
+          ColonColon.setKind(tok::colon);
+          PP.EnterToken(Tok);
+          PP.EnterToken(ColonColon);
+          Tok = Identifier;
+          break;
+        }
         SS.SetInvalid(SourceRange(IdLoc, CCLoc));
----------------
I think it'd be cleaner to pass a `bool *CorrectToColon` into `ActOnCXXNestedNameSpecifier`, and issue the diagnostics there (so this diagnostic is not split into two disparate places). Then, if the flag is set, just fix up the token stream here.


http://llvm-reviews.chandlerc.com/D2870



More information about the cfe-commits mailing list