Provide fixit if unscoped enumeration is used in nested name specifier. This fixes PR16951.

[sepavloff]

Updated patch to address reviewer's notes. Added handling of Microsoft extension.

[rsmith]

This patch does not seem to be correct. Using an enumerator as a nested-name-specifier is a C++11 extension. MSVC enables C++11 by default. The only bug I can see here is that our diagnostic is misworded: in C++11, we should say "%0 is not a class, namespace, or enumeration" (remove the word "scoped"), and in C++98, we should say "%0 is not a class or namespace"

[sepavloff]

MSVC supported using an enumerator as a nested-name-specifier for a long time. For instance, VS2005 compiled the code:

enum ABCD { A, B, C };

int main() {
  return ABCD::A;
}

without errors:

C:\Tools>cl tt.cpp
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 14.00.50727.42 for 80x86
Copyright (C) Microsoft Corporation.  All rights reserved.

tt.cpp
tt.cpp(4) : warning C4482: nonstandard extension used: enum 'ABCD' used in qualified name
Microsoft (R) Incremental Linker Version 8.00.50727.42
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:tt.exe
tt.obj

C:\Tools>

VS2010 compiles this code without warnings by default. It is reasonable to accept this code in Microsoft mode without warnings by default, isn't it?

However, complaining on using extension make the patch simpler, will update it.

[sepavloff]

Updated patch.

Instead of providing removal fixit, issue extension warning.

[rsmith]

LGTM with a couple of tweaks. (We should also fix the "is not a class, namespace, or scoped enumeration" diagnostic to remove the word "scoped", feel free to go ahead and commit a patch for that.)

include/clang/Basic/DiagnosticSemaKinds.td

1232 ↗

(On Diff #18311)

Remove the "unscoped" here. C++98 didn't have scoped enumerations, and in any case, this diagnostic is not specific to unscoped enumerations.

lib/Sema/SemaCXXScopeSpec.cpp

286–287

s/may be used in extended syntax/is accepted as an extension/

include/clang/Basic/DiagnosticParseKinds.td

def note_misplaced_ellipsis_vararg_add_comma : Note<
   "insert ',' before '...' to silence this warning">;
 def ext_abstract_pack_declarator_parens : ExtWarn<
   "ISO C++11 requires a parenthesized pack declaration to have a name">,
   InGroup<DiagGroup<"anonymous-pack-parens">>;
 def err_function_is_not_record : Error<
   "unexpected %0 in function call; perhaps remove the %0?">;
 def err_super_in_using_declaration : Error<
   "'__super' cannot be used with a using declaration">;
+def err_nested_name_spec_is_enum : Error<
+  "unscoped enumeration name shall not be specified in nested name specifier">;
+def err_expected_class_or_namespace_or_enum : Error<"%0 is not a class"
+  "%select{ or namespace|, namespace, or scoped enumeration}1">;
 
 // C++ derived classes
 def err_dup_virtual : Error<"duplicate 'virtual' in base specifier">;
 
 // C++ operator overloading
 def err_literal_operator_string_prefix : Error<
   "string literal after 'operator' cannot have an encoding prefix">;
 def err_literal_operator_string_not_empty : Error<

include/clang/Sema/Sema.h

public:
   bool isAcceptableNestedNameSpecifier(const NamedDecl *SD);
   NamedDecl *FindFirstQualifierInScope(Scope *S, NestedNameSpecifier *NNS);
 
   bool isNonTypeNestedNameSpecifier(Scope *S, CXXScopeSpec &SS,
                                     SourceLocation IdLoc,
                                     IdentifierInfo &II,
                                     ParsedType ObjectType);
 
+  /// \brief Collects information that may be used to recover erroneous nested
+  /// name specifier.
+  struct NameSpecRecoveryInfo {
+    bool CorrectToColon; //> suggestions to replace '::' -> ':' are allowed
+    bool CorrectionToColonRequested; //> Replacement '::' -> ':' recovers error
+    TypeDecl *FoundDecl; //> Declaration if found for name spec
+
+    NameSpecRecoveryInfo(bool CorrectColon) {
+      clear();
+      CorrectToColon = CorrectColon;
+    }
+    void clear() {
+      CorrectToColon = CorrectionToColonRequested = false;
+      FoundDecl = nullptr;
+    }
+  };
+
   bool BuildCXXNestedNameSpecifier(Scope *S,
                                    IdentifierInfo &Identifier,
                                    SourceLocation IdentifierLoc,
                                    SourceLocation CCLoc,
                                    QualType ObjectType,
                                    bool EnteringContext,
                                    CXXScopeSpec &SS,
                                    NamedDecl *ScopeLookupResult,
                                    bool ErrorRecoveryLookup,
-                                   bool *IsCorrectedToColon = nullptr);
+                                   NameSpecRecoveryInfo *RecovInfo = nullptr);
 
   /// \brief The parser has parsed a nested-name-specifier 'identifier::'.
   ///
   /// \param S The scope in which this nested-name-specifier occurs.
   ///
   /// \param Identifier The identifier preceding the '::'.
   ///
   /// \param IdentifierLoc The location of the identifier.
public:
   /// \param SS The nested-name-specifier, which is both an input
   /// parameter (the nested-name-specifier before this type) and an
   /// 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 '::'.
+  /// \param RecovInfo If not null, points to a structure that is contains info
+  /// for error recovery.
   ///
   /// \returns true if an error occurred, false otherwise.
   bool ActOnCXXNestedNameSpecifier(Scope *S,
                                    IdentifierInfo &Identifier,
                                    SourceLocation IdentifierLoc,
                                    SourceLocation CCLoc,
                                    ParsedType ObjectType,
                                    bool EnteringContext,
                                    CXXScopeSpec &SS,
                                    bool ErrorRecoveryLookup = false,
-                                   bool *IsCorrectedToColon = nullptr);
+                                   NameSpecRecoveryInfo *RecovInfo = nullptr);
 
   ExprResult ActOnDecltypeExpression(Expr *E);
 
   bool ActOnCXXNestedNameSpecifierDecltype(CXXScopeSpec &SS,
                                            const DeclSpec &DS,
                                            SourceLocation ColonColonLoc);
 
   bool IsInvalidUnlessNestedName(Scope *S, CXXScopeSpec &SS,

lib/Parse/ParseExprCXX.cpp

if (Next.is(tok::coloncolon)) {
       SourceLocation IdLoc = ConsumeToken();
       assert((Tok.is(tok::coloncolon) || Tok.is(tok::colon)) &&
              "NextToken() not working properly!");
       Token ColonColon = Tok;
       SourceLocation CCLoc = ConsumeToken();
 
       CheckForLParenAfterColonColon();
 
-      bool IsCorrectedToColon = false;
-      bool *CorrectionFlagPtr = ColonIsSacred ? &IsCorrectedToColon : nullptr;
+      Sema::NameSpecRecoveryInfo RecovInfo(ColonIsSacred);
       if (Actions.ActOnCXXNestedNameSpecifier(getCurScope(), II, IdLoc, CCLoc,
                                               ObjectType, EnteringContext, SS,
-                                              false, CorrectionFlagPtr)) {
+                                              false, &RecovInfo)) {
         // Identifier is not recognized as a nested name, but we can have
         // mistyped '::' instead of ':'.
-        if (CorrectionFlagPtr && IsCorrectedToColon) {
+        if (RecovInfo.CorrectToColon && RecovInfo.CorrectionToColonRequested) {
           ColonColon.setKind(tok::colon);
           PP.EnterToken(Tok);
           PP.EnterToken(ColonColon);
           Tok = Identifier;
           break;
         }
+        if (RecovInfo.FoundDecl) {
+          // The current identifier does not represents entity that may be used
+          // as nested name spec, it nevertheless resolves to a declaration. See
+          // if we can do error recover here.
+          bool ErrorReported = false;
+          if (EnumDecl *ED = dyn_cast<EnumDecl>(RecovInfo.FoundDecl)) {
+            // Maybe non scoped enum is used as scoped?  If so this identifier
+            // must be followed by another, which is corresponding enumerator.
+            if (!ED->isScoped() && Tok.is(tok::identifier) &&
+                NextToken().isNot(tok::coloncolon)) {
+              IdentifierInfo *Id = Tok.getIdentifierInfo();
+              for (auto Enumerator : ED->enumerators()) {
+                if (Enumerator->getName().equals(Id->getName())) {
+                  // We can recover parsing by throwing away enumeration name.
+                  Diag(IdLoc, diag::err_nested_name_spec_is_enum) <<
+                      FixItHint::CreateRemoval(SourceRange(IdLoc, CCLoc));
+                  ErrorReported = true;
+                  break;
+                }
+              }
+            }
+          }
+          if (!ErrorReported) {
+            Diag(IdLoc, diag::err_expected_class_or_namespace_or_enum)
+              << QualType(RecovInfo.FoundDecl->getTypeForDecl(), 0)
+              << getLangOpts().CPlusPlus;
+          }
+        }
         SS.SetInvalid(SourceRange(IdLoc, CCLoc));
       }
       HasScopeSpecifier = true;
       continue;
     }
 
     CheckForTemplateAndDigraph(Next, ObjectType, EnteringContext, II, SS);
 

lib/Sema/SemaCXXScopeSpec.cpp

bool Sema::ActOnSuperScopeSpecifier(SourceLocation SuperLoc,
 
   SS.MakeSuper(Context, RD, SuperLoc, ColonColonLoc);
   return false;
 }
 
 /// \brief Determines whether the given declaration is an valid acceptable
 /// result for name lookup of a nested-name-specifier.
 bool Sema::isAcceptableNestedNameSpecifier(const NamedDecl *SD) {
   if (!SD)
     return false;
 
   // Namespace and namespace aliases are fine.
   if (isa<NamespaceDecl>(SD) || isa<NamespaceAliasDecl>(SD))
     return true;
 
   if (!isa<TypeDecl>(SD))
     return false;
 

 /// \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 '::'.
+/// \param RecovInfo If not null, points to a structure that contains info
+///        for error recovery.
 ///
 /// 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.
 ///
 /// If ErrorRecoveryLookup is true, then this call is used to improve error
 /// recovery.  This means that it should not emit diagnostics, it should
 /// just return true on failure.  It also means it should only return a valid
 /// scope if it *knows* that the result is correct.  It should not return in a
 /// dependent context, for example. Nor will it extend \p SS with the scope
 /// specifier.
 bool Sema::BuildCXXNestedNameSpecifier(Scope *S,
                                        IdentifierInfo &Identifier,
                                        SourceLocation IdentifierLoc,
                                        SourceLocation CCLoc,
                                        QualType ObjectType,
                                        bool EnteringContext,
                                        CXXScopeSpec &SS,
                                        NamedDecl *ScopeLookupResult,
                                        bool ErrorRecoveryLookup,
-                                       bool *IsCorrectedToColon) {
+                                       NameSpecRecoveryInfo *RecovInfo) {
   LookupResult Found(*this, &Identifier, IdentifierLoc, 
                      LookupNestedNameSpecifierName);
 
   // Determine where to perform name lookup
   DeclContext *LookupCtx = nullptr;
   bool isDependent = false;
-  if (IsCorrectedToColon)
-    *IsCorrectedToColon = false;
+  if (RecovInfo) {
+    RecovInfo->CorrectionToColonRequested = false;
+    RecovInfo->FoundDecl = nullptr;
+  }
   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.
     assert(!SS.isSet() && "ObjectType and scope specifier cannot coexist");
     LookupCtx = computeDeclContext(ObjectType);
     isDependent = ObjectType->isDependentType();
   } else if (SS.isSet()) {
     // This nested-name-specifier occurs after another nested-name-specifier,
if (Found.empty() && !ErrorRecoveryLookup) {
     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;
+      if (RecovInfo && RecovInfo->CorrectToColon) {
+        RecovInfo->CorrectionToColonRequested = 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.
if (DC->isDependentContext() && DC->isFunctionOrMethod()) {
             << &Identifier << ContainingClass;
         SS.Extend(Context, &Identifier, IdentifierLoc, CCLoc);
         return false;
       }
     }
   }
 
   if (!Found.empty()) {
-    if (TypeDecl *TD = Found.getAsSingle<TypeDecl>())
+    if (TypeDecl *TD = Found.getAsSingle<TypeDecl>()) {
+      if (RecovInfo)
+        RecovInfo->FoundDecl = TD;
+      else
         Diag(IdentifierLoc, diag::err_expected_class_or_namespace)
           << QualType(TD->getTypeForDecl(), 0) << getLangOpts().CPlusPlus;
-    else {
+    } else {
       Diag(IdentifierLoc, diag::err_expected_class_or_namespace)
           << &Identifier << getLangOpts().CPlusPlus;
       if (NamedDecl *ND = Found.getAsSingle<NamedDecl>())
         Diag(ND->getLocation(), diag::note_entity_declared_at) << &Identifier;
     }
   } else if (SS.isSet())
     Diag(IdentifierLoc, diag::err_no_member) << &Identifier << LookupCtx
                                              << SS.getRange();
   else
     Diag(IdentifierLoc, diag::err_undeclared_var_use) << &Identifier;
 
   return true;
 }
 
 bool Sema::ActOnCXXNestedNameSpecifier(Scope *S,
                                        IdentifierInfo &Identifier,
                                        SourceLocation IdentifierLoc,
                                        SourceLocation CCLoc,
                                        ParsedType ObjectType,
                                        bool EnteringContext,
                                        CXXScopeSpec &SS,
                                        bool ErrorRecoveryLookup,
-                                       bool *IsCorrectedToColon) {
+                                       NameSpecRecoveryInfo *RecovInfo) {
   if (SS.isInvalid())
     return true;
 
   return BuildCXXNestedNameSpecifier(S, Identifier, IdentifierLoc, CCLoc,
                                      GetTypeFromParser(ObjectType),
                                      EnteringContext, SS, 
                                      /*ScopeLookupResult=*/nullptr, false,
-                                     IsCorrectedToColon);
+                                     RecovInfo);
 }
 
 bool Sema::ActOnCXXNestedNameSpecifierDecltype(CXXScopeSpec &SS,
                                                const DeclSpec &DS,
                                                SourceLocation ColonColonLoc) {
   if (SS.isInvalid() || DS.getTypeSpecType() == DeclSpec::TST_error)
     return true;
 

test/SemaCXX/nested-name-spec.cpp

namespace E {
   int X = 5;
   
   namespace Nested {
     enum E {
       X = 0
     };
 
     void f() {
-      return E::X; // expected-error{{'E::Nested::E' is not a class, namespace, or scoped enumeration}}
+      return E::X; // expected-error{{unscoped enumeration name shall not be specified in nested name specifier}}
     }
   }
 }
 
 
 class Operators {
   Operators operator+(const Operators&) const; // expected-note{{member declaration does not match because it is const qualified}}
   operator bool();

 struct S7b {
   T1<B1:B2>::C1 m1 : T1<B1::B2>::N1;  // expected-error{{unexpected ':' in nested name specifier; did you mean '::'?}}
 };
 struct S7c {
   T1<B1::B2>::C1 m1 : T1<B1:B2>::N1;  // expected-error{{unexpected ':' in nested name specifier; did you mean '::'?}}
 };
 
 }
+
+
+namespace PR16951 {
+  namespace ns {
+    enum an_enumeration {
+      ENUMERATOR
+    };
+  }
+
+  int x1 = ns::an_enumeration::ENUMERATOR; // expected-error{{unscoped enumeration name shall not be specified in nested name specifier}}
+  int x2 = ns::an_enumeration::ENUMERATOR::vvv; // expected-error{{'PR16951::ns::an_enumeration' is not a class, namespace, or scoped enumeration}}
+}