Improve error recovery around colon.

Parse of nested name spacifier is modified so that it properly recovers
if colon is mistyped as double colon in case statement.
This patch fixes PR15133.

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


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

10 files changed, 200 insertions, 33 deletions

diff --git a/include/clang/Basic/DiagnosticParseKinds.td b/include/clang/Basic/DiagnosticParseKinds.td
index 3efac016ad..04f4e5e1fc 100644
--- a/include/clang/Basic/DiagnosticParseKinds.td
+++ b/include/clang/Basic/DiagnosticParseKinds.td
@@ -339,8 +339,10 @@ def err_templated_using_directive : Error<
   "cannot template a using directive">;
 def err_templated_using_declaration : Error<
   "cannot template a using declaration">;
-def err_unexected_colon_in_nested_name_spec : Error<
+def err_unexpected_colon_in_nested_name_spec : Error<
   "unexpected ':' in nested name specifier; did you mean '::'?">;
+def err_unexpected_token_in_nested_name_spec : Error<
+  "'%0' cannot be a part of nested name specifier; did you mean ':'?">;
 def err_bool_redeclaration : Error<
   "redeclaration of C++ built-in type 'bool'">;
 def ext_c11_static_assert : Extension<
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 0eefcd70f0..a1a5f5a892 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1169,7 +1169,10 @@ def err_incomplete_type : Error<
 def warn_cxx98_compat_enum_nested_name_spec : Warning<
   "enumeration type in nested name specifier is incompatible with C++98">,
   InGroup<CXX98Compat>, DefaultIgnore;
-  
+def err_nested_name_spec_is_not_class : Error<
+  "%0 cannot appear before '::' because it is not a class"
+  "%select{ or namespace|, namespace, or scoped enumeration}1; did you mean ':'?">;
+
 // C++ class members
 def err_storageclass_invalid_for_member : Error<
   "storage class specified for a member declaration">;
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index a7b6e787f7..f53f73642a 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -4424,7 +4424,8 @@ public:
                                    bool EnteringContext,
                                    CXXScopeSpec &SS,
                                    NamedDecl *ScopeLookupResult,
-                                   bool ErrorRecoveryLookup);
+                                   bool ErrorRecoveryLookup,
+                                   bool *IsCorrectedToColon = 0);
 
   /// \brief The parser has parsed a nested-name-specifier 'identifier::'.
   ///
@@ -4447,6 +4448,13 @@ public:
   /// output parameter (containing the full nested-name-specifier,
   /// including this new type).
   ///
+  /// \param ErrorRecoveryLookup If true, then this method is called to improve
+  /// error recovery. In this case do not emit error message.
+  ///
+  /// \param IsCorrectedToColon If not null, suggestions to replace '::' -> ':'
+  /// are allowed.  The bool value pointed by this parameter is set to 'true'
+  /// if the identifier is treated as if it was followed by ':', not '::'.
+  ///
   /// \returns true if an error occurred, false otherwise.
   bool ActOnCXXNestedNameSpecifier(Scope *S,
                                    IdentifierInfo &Identifier,
@@ -4454,7 +4462,9 @@ public:
                                    SourceLocation CCLoc,
                                    ParsedType ObjectType,
                                    bool EnteringContext,
-                                   CXXScopeSpec &SS);
+                                   CXXScopeSpec &SS,
+                                   bool ErrorRecoveryLookup = false,
+                                   bool *IsCorrectedToColon = 0);
 
   ExprResult ActOnDecltypeExpression(Expr *E);
 
diff --git a/lib/Parse/ParseExprCXX.cpp b/lib/Parse/ParseExprCXX.cpp
index 6495d193f7..614949c935 100644
--- a/lib/Parse/ParseExprCXX.cpp
+++ b/lib/Parse/ParseExprCXX.cpp
@@ -365,15 +365,15 @@ bool Parser::ParseOptionalCXXScopeSpecifier(CXXScopeSpec &SS,
 
       // Consume the template-id token.
       ConsumeToken();
-      
+
       assert(Tok.is(tok::coloncolon) && "NextToken() not working properly!");
       SourceLocation CCLoc = ConsumeToken();
 
       HasScopeSpecifier = true;
-      
+
       ASTTemplateArgsPtr TemplateArgsPtr(TemplateId->getTemplateArgs(),
                                          TemplateId->NumArgs);
-      
+
       if (Actions.ActOnCXXNestedNameSpecifier(getCurScope(),
                                               SS,
                                               TemplateId->TemplateKWLoc,
@@ -393,7 +393,6 @@ bool Parser::ParseOptionalCXXScopeSpecifier(CXXScopeSpec &SS,
       continue;
     }
 
-
     // The rest of the nested-name-specifier possibilities start with
     // tok::identifier.
     if (Tok.isNot(tok::identifier))
@@ -418,9 +417,8 @@ bool Parser::ParseOptionalCXXScopeSpecifier(CXXScopeSpec &SS,
           // error, but they probably meant something else strange so don't
           // recover like this.
           PP.LookAhead(1).is(tok::identifier)) {
-        Diag(Next, diag::err_unexected_colon_in_nested_name_spec)
+        Diag(Next, diag::err_unexpected_colon_in_nested_name_spec)
           << FixItHint::CreateReplacement(Next.getLocation(), "::");
-        
         // Recover as if the user wrote '::'.
         Next.setKind(tok::coloncolon);
       }
@@ -434,23 +432,52 @@ bool Parser::ParseOptionalCXXScopeSpecifier(CXXScopeSpec &SS,
         return false;
       }
 
+      if (ColonIsSacred) {
+        const Token &Next2 = GetLookAheadToken(2);
+        if (Next2.is(tok::kw_private) || Next2.is(tok::kw_protected) ||
+            Next2.is(tok::kw_public) || Next2.is(tok::kw_virtual)) {
+          Diag(Next2, diag::err_unexpected_token_in_nested_name_spec)
+              << Next2.getName()
+              << FixItHint::CreateReplacement(Next.getLocation(), ":");
+          Token ColonColon;
+          PP.Lex(ColonColon);
+          ColonColon.setKind(tok::colon);
+          PP.EnterToken(ColonColon);
+          break;
+        }
+      }
+
       if (LastII)
         *LastII = &II;
 
       // We have an identifier followed by a '::'. Lookup this name
       // as the name in a nested-name-specifier.
+      Token Identifier = Tok;
       SourceLocation IdLoc = ConsumeToken();
       assert((Tok.is(tok::coloncolon) || Tok.is(tok::colon)) &&
              "NextToken() not working properly!");
+      Token ColonColon = Tok;
       SourceLocation CCLoc = ConsumeToken();
 
       CheckForLParenAfterColonColon();
 
-      HasScopeSpecifier = true;
+      bool IsCorrectedToColon = false;
+      bool *CorrectionFlagPtr = ColonIsSacred ? &IsCorrectedToColon : 0;
       if (Actions.ActOnCXXNestedNameSpecifier(getCurScope(), II, IdLoc, CCLoc,
-                                              ObjectType, EnteringContext, SS))
+                                              ObjectType, EnteringContext, SS,
+                                              false, CorrectionFlagPtr)) {
+        // Identifier is not recognized as a nested name, but we can have
+        // mistyped '::' instead of ':'.
+        if (CorrectionFlagPtr && IsCorrectedToColon) {
+          ColonColon.setKind(tok::colon);
+          PP.EnterToken(Tok);
+          PP.EnterToken(ColonColon);
+          Tok = Identifier;
+          break;
+        }
         SS.SetInvalid(SourceRange(IdLoc, CCLoc));
-      
+      }
+      HasScopeSpecifier = true;
       continue;
     }
 
diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
index 0fbb764fde..df66240ed7 100644
--- a/lib/Parse/ParseStmt.cpp
+++ b/lib/Parse/ParseStmt.cpp
@@ -643,8 +643,9 @@ StmtResult Parser::ParseCaseStatement(bool MissingCase, ExprResult Expr) {
     ColonProtection.restore();
 
     if (TryConsumeToken(tok::colon, ColonLoc)) {
-    } else if (TryConsumeToken(tok::semi, ColonLoc)) {
-      // Treat "case blah;" as a typo for "case blah:".
+    } else if (TryConsumeToken(tok::semi, ColonLoc) ||
+               TryConsumeToken(tok::coloncolon, ColonLoc)) {
+      // Treat "case blah;" or "case blah::" as a typo for "case blah:".
       Diag(ColonLoc, diag::err_expected_after)
           << "'case'" << tok::colon
           << FixItHint::CreateReplacement(ColonLoc, ":");
diff --git a/lib/Parse/ParseTemplate.cpp b/lib/Parse/ParseTemplate.cpp
index 777fd01aa2..e81c41da67 100644
--- a/lib/Parse/ParseTemplate.cpp
+++ b/lib/Parse/ParseTemplate.cpp
@@ -1161,6 +1161,7 @@ bool
 Parser::ParseTemplateArgumentList(TemplateArgList &TemplateArgs) {
   // Template argument lists are constant-evaluation contexts.
   EnterExpressionEvaluationContext EvalContext(Actions,Sema::ConstantEvaluated);
+  ColonProtectionRAIIObject ColonProtection(*this, false);
 
   do {
     ParsedTemplateArgument Arg = ParseTemplateArgument();
diff --git a/lib/Sema/SemaCXXScopeSpec.cpp b/lib/Sema/SemaCXXScopeSpec.cpp
index ce47cfffa8..7d038f72e9 100644
--- a/lib/Sema/SemaCXXScopeSpec.cpp
+++ b/lib/Sema/SemaCXXScopeSpec.cpp
@@ -377,6 +377,28 @@ class NestedNameSpecifierValidatorCCC : public CorrectionCandidateCallback {
 /// by ActOnCXXNestedNameSpecifier.
 ///
 /// This routine differs only slightly from ActOnCXXNestedNameSpecifier, in
+/// that it contains an extra parameter \p ScopeLookupResult.
+///
+/// \param S Scope in which the nested-name-specifier occurs.
+/// \param Identifier Identifier in the sequence "identifier" "::".
+/// \param IdentifierLoc Location of the \p Identifier.
+/// \param CCLoc Location of "::" following Identifier.
+/// \param ObjectType Type of postfix expression if the nested-name-specifier
+///        occurs in construct like: <tt>ptr->nns::f</tt>.
+/// \param EnteringContext If true, enter the context specified by the
+///        nested-name-specifier.
+/// \param SS Optional nested name specifier preceding the identifier.
+/// \param ScopeLookupResult Provides the result of name lookup within the
+///        scope of the nested-name-specifier that was computed at template
+///        definition time.
+/// \param ErrorRecoveryLookup Specifies if the method is called to improve
+///        error recovery and what kind of recovery is performed.
+/// \param IsCorrectedToColon If not null, suggestion of replace '::' -> ':'
+///        are allowed.  The bool value pointed by this parameter is set to
+///       'true' if the identifier is treated as if it was followed by ':',
+///        not '::'.
+///
+/// This routine differs only slightly from ActOnCXXNestedNameSpecifier, in
 /// that it contains an extra parameter \p ScopeLookupResult, which provides
 /// the result of name lookup within the scope of the nested-name-specifier
 /// that was computed at template definition time.
@@ -395,13 +417,16 @@ bool Sema::BuildCXXNestedNameSpecifier(Scope *S,
                                        bool EnteringContext,
                                        CXXScopeSpec &SS,
                                        NamedDecl *ScopeLookupResult,
-                                       bool ErrorRecoveryLookup) {
+                                       bool ErrorRecoveryLookup,
+                                       bool *IsCorrectedToColon) {
   LookupResult Found(*this, &Identifier, IdentifierLoc, 
                      LookupNestedNameSpecifierName);
 
   // Determine where to perform name lookup
   DeclContext *LookupCtx = 0;
   bool isDependent = false;
+  if (IsCorrectedToColon)
+    *IsCorrectedToColon = false;
   if (!ObjectType.isNull()) {
     // This nested-name-specifier occurs in a member access expression, e.g.,
     // x->B::f, and we are looking into the type of the object.
@@ -416,7 +441,6 @@ bool Sema::BuildCXXNestedNameSpecifier(Scope *S,
     Found.setContextRange(SS.getRange());
   }
 
-
   bool ObjectTypeSearchedInScope = false;
   if (LookupCtx) {
     // Perform "qualified" name lookup into the declaration context we
@@ -473,17 +497,48 @@ bool Sema::BuildCXXNestedNameSpecifier(Scope *S,
     // Don't speculate if we're just trying to improve error recovery.
     if (ErrorRecoveryLookup)
       return true;
-    
+
     // We were not able to compute the declaration context for a dependent
     // base object type or prior nested-name-specifier, so this
     // nested-name-specifier refers to an unknown specialization. Just build
     // a dependent nested-name-specifier.
     SS.Extend(Context, &Identifier, IdentifierLoc, CCLoc);
     return false;
-  } 
-  
+  }
+
   // FIXME: Deal with ambiguities cleanly.
 
+  if (Found.empty() && !ErrorRecoveryLookup) {
+    // If identifier is not found as class-name-or-namespace-name, but is found
+    // as other entity, don't look for typos.
+    LookupResult R(*this, Found.getLookupNameInfo(), LookupOrdinaryName);
+    if (LookupCtx)
+      LookupQualifiedName(R, LookupCtx);
+    else if (S && !isDependent)
+      LookupName(R, S);
+    if (!R.empty()) {
+      // The identifier is found in ordinary lookup. If correction to colon is
+      // allowed, suggest replacement to ':'.
+      if (IsCorrectedToColon) {
+        *IsCorrectedToColon = true;
+        Diag(CCLoc, diag::err_nested_name_spec_is_not_class)
+            << &Identifier << getLangOpts().CPlusPlus
+            << FixItHint::CreateReplacement(CCLoc, ":");
+        if (NamedDecl *ND = R.getAsSingle<NamedDecl>())
+          Diag(ND->getLocation(), diag::note_declared_at);
+        return true;
+      }
+      // Replacement '::' -> ':' is not allowed, just issue respective error.
+      Diag(R.getNameLoc(), diag::err_expected_class_or_namespace)
+          << &Identifier << getLangOpts().CPlusPlus;
+      if (NamedDecl *ND = R.getAsSingle<NamedDecl>())
+        Diag(ND->getLocation(),
+             diag::note_expected_class_or_namespace_declared_here)
+            << &Identifier;
+      return true;
+    }
+  }
+
   if (Found.empty() && !ErrorRecoveryLookup && !getLangOpts().MSVCCompat) {
     // We haven't found anything, and we're not recovering from a
     // different kind of error, so look for typos.
@@ -543,8 +598,8 @@ bool Sema::BuildCXXNestedNameSpecifier(Scope *S,
            !Context.hasSameType(
                             Context.getTypeDeclType(cast<TypeDecl>(OuterDecl)),
                                Context.getTypeDeclType(cast<TypeDecl>(SD))))) {
-         if (ErrorRecoveryLookup)
-           return true;
+        if (ErrorRecoveryLookup)
+          return true;
 
          Diag(IdentifierLoc, 
               diag::err_nested_name_member_ref_lookup_ambiguous)
@@ -558,7 +613,7 @@ bool Sema::BuildCXXNestedNameSpecifier(Scope *S,
        }
     }
 
-    // If we're just performing this lookup for error-recovery purposes, 
+    // If we're just performing this lookup for error-recovery purposes,
     // don't extend the nested-name-specifier. Just return now.
     if (ErrorRecoveryLookup)
       return false;
@@ -681,14 +736,17 @@ bool Sema::ActOnCXXNestedNameSpecifier(Scope *S,
                                        SourceLocation CCLoc,
                                        ParsedType ObjectType,
                                        bool EnteringContext,
-                                       CXXScopeSpec &SS) {
+                                       CXXScopeSpec &SS,
+                                       bool ErrorRecoveryLookup,
+                                       bool *IsCorrectedToColon) {
   if (SS.isInvalid())
     return true;
-  
+
   return BuildCXXNestedNameSpecifier(S, Identifier, IdentifierLoc, CCLoc,
                                      GetTypeFromParser(ObjectType),
                                      EnteringContext, SS, 
-                                     /*ScopeLookupResult=*/0, false);
+                                     /*ScopeLookupResult=*/0, false,
+                                     IsCorrectedToColon);
 }
 
 bool Sema::ActOnCXXNestedNameSpecifierDecltype(CXXScopeSpec &SS,
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index cc461ef141..948c58030a 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -584,16 +584,15 @@ Sema::NameClassification Sema::ClassifyName(Scope *S,
                                             CorrectionCandidateCallback *CCC) {
   DeclarationNameInfo NameInfo(Name, NameLoc);
   ObjCMethodDecl *CurMethod = getCurMethodDecl();
-  
+
   if (NextToken.is(tok::coloncolon)) {
     BuildCXXNestedNameSpecifier(S, *Name, NameLoc, NextToken.getLocation(),
                                 QualType(), false, SS, 0, false);
-    
   }
-      
+
   LookupResult Result(*this, Name, NameLoc, LookupOrdinaryName);
   LookupParsedName(Result, S, &SS, !CurMethod);
-  
+
   // Perform lookup for Objective-C instance variables (including automatically 
   // synthesized instance variables), if we're in an Objective-C method.
   // FIXME: This lookup really, really needs to be folded in to the normal
diff --git a/test/Parser/recovery.cpp b/test/Parser/recovery.cpp
index cb1979f9ff..5608b50468 100644
--- a/test/Parser/recovery.cpp
+++ b/test/Parser/recovery.cpp
@@ -135,3 +135,70 @@ enum class EnumID {};
 template <typename> struct TempID;
 template <> struct TempID<BadType> : BadType, EnumID::Garbage; // expected-error{{use of undeclared identifier 'BadType'}}
 }
+
+namespace pr15133 {
+  namespace ns {
+    const int V1 = 1;   // expected-note {{declared here}}
+  }
+  struct C1 {
+    enum E1 { V2 = 2 }; // expected-note {{declared here}}
+    static const int V3 = 3; // expected-note {{declared here}}
+  };
+  enum E2 {
+    V4 = 4,   // expected-note {{declared here}}
+    V6        // expected-note {{declared here}}
+  };
+  enum class EC3 { V0 = 0, V5 = 5 }; // expected-note {{declared here}}
+  void func_3();
+
+  void func_1(int x) {
+    switch(x) {
+    case 0: break;
+    case ns::V1:: break; // expected-error{{'V1' cannot appear before '::' because it is not a class, namespace, or scoped enumeration; did you mean ':'?}}
+    case C1::V2:: break; // expected-error{{'V2' cannot appear before '::' because it is not a class, namespace, or scoped enumeration; did you mean ':'?}}
+    case C1::V3:: break; // expected-error{{'V3' cannot appear before '::' because it is not a class, namespace, or scoped enumeration; did you mean ':'?}}
+    case V4:: break; // expected-error{{'V4' cannot appear before '::' because it is not a class, namespace, or scoped enumeration; did you mean ':'?}}
+    case V6:: func_3();   // expected-error{{'V6' cannot appear before '::' because it is not a class, namespace, or scoped enumeration; did you mean ':'?}}
+    }
+  }
+  void func_2(EC3 x) {
+    switch(x) {
+    case EC3::V0:  break;
+    case EC3::V5:: break; // expected-error{{'V5' cannot appear before '::' because it is not a class, namespace, or scoped enumeration; did you mean ':'?}}
+    }
+  }
+
+  template<class T> struct TS1 {
+    typedef int A;
+  };
+  template<class T> void func(int x) {
+    switch(x) {
+    case TS1<T>::A:: break;  // expected-error{{expected unqualified-id}}
+    }
+  };
+  void mainf() {
+    func<int>(1);
+  }
+
+  struct S {
+    static int n;  // expected-note{{declared here}}
+    int nn;        // expected-note 2 {{declared here}}
+  };
+
+  int func_3(int x) {
+    return x ? S::n :: 0;  // expected-error{{'n' cannot appear before '::' because it is not a class, namespace, or scoped enumeration; did you mean ':'?}}
+  }
+  int func_4(int x, S &s) {
+    return x ? s.nn :: x;  // expected-error{{'nn' cannot appear before '::' because it is not a class, namespace, or scoped enumeration; did you mean ':'?}}
+  }
+  int func_5(int x, S &s) {
+    return x ? s.nn :: S::n;  // expected-error{{'nn' cannot appear before '::' because it is not a class, namespace, or scoped enumeration; did you mean ':'?}}
+  }
+
+  struct S2 {
+    struct S3;
+  };
+
+  struct S2 :: S3 :: public S2 {  // expected-error{{'public' cannot be a part of nested name specifier; did you mean ':'?}}
+  };
+}
diff --git a/test/SemaCXX/nested-name-spec.cpp b/test/SemaCXX/nested-name-spec.cpp
index 8587e70158..bfbf9c4135 100644
--- a/test/SemaCXX/nested-name-spec.cpp
+++ b/test/SemaCXX/nested-name-spec.cpp
@@ -8,13 +8,12 @@ namespace A {
     static int Ag1();
     static int Ag2();
   };
-  int ax;
+  int ax; // expected-note {{'ax' declared here}}
   void Af();
 }
 
 A:: ; // expected-error {{expected unqualified-id}}
-// FIXME: there is a member 'ax'; it's just not a class.
-::A::ax::undef ex3; // expected-error {{no member named 'ax'}}
+::A::ax::undef ex3; // expected-error {{'ax' is not a class, namespace, or scoped enumeration}}
 A::undef1::undef2 ex4; // expected-error {{no member named 'undef1'}}
 
 int A::C::Ag1() { return 0; }