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

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/DiagnosticSemaKinds.td

 def err_incomplete_type : Error<
   "incomplete type %0 where a complete type is required">;
 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 ':'?">;
+def err_nested_name_spec_is_enum : Error<
+  "unscoped enumeration name cannot be specified as a nested name specifier">;
 
 // C++ class members
 def err_storageclass_invalid_for_member : Error<
   "storage class specified for a member declaration">;
 def err_mutable_function : Error<"'mutable' cannot be applied to functions">;
 def err_mutable_reference : Error<"'mutable' cannot be applied to references">;
 def err_mutable_const : Error<"'mutable' and 'const' cannot be mixed">;
 def err_mutable_nonmember : Error<

 def ext_forward_ref_enum : Extension<
   "ISO C forbids forward references to 'enum' types">;
 def err_forward_ref_enum : Error<
   "ISO C++ forbids forward references to 'enum' types">;
 def ext_ms_forward_ref_enum : Extension<
   "forward references to 'enum' types are a Microsoft extension">, InGroup<Microsoft>;
 def ext_forward_ref_enum_def : Extension<
   "redeclaration of already-defined enum %0 is a GNU extension">, InGroup<GNURedeclaredEnum>;
-  
+def ext_ms_enum_in_nested_name_spec : Extension<
+  "using 'enum' types in nested name specifier is a Microsoft extension">,
+   InGroup<Microsoft>;
 def err_redefinition_of_enumerator : Error<"redefinition of enumerator %0">;
 def err_duplicate_member : Error<"duplicate member %0">;
 def err_misplaced_ivar : Error<
   "instance variables may not be placed in %select{categories|class extension}0">;
 def warn_ivars_in_interface : Warning<
   "declaration of instance variables in the interface is deprecated">,
   InGroup<DiagGroup<"objc-interface-ivars">>, DefaultIgnore;
 def ext_enum_value_not_int : Extension<
   "ISO C restricts enumerator values to range of 'int' (%0 is too "
   "%select{small|large}1)">;
 def ext_enum_too_large : ExtWarn<
   "enumeration values exceed range of largest integer">, InGroup<EnumTooLarge>;
 def ext_enumerator_increment_too_large : ExtWarn<
   "incremented enumerator value %0 is not representable in the "
   "largest integer type">, InGroup<EnumTooLarge>;
 def warn_flag_enum_constant_out_of_range : Warning<
   "enumeration value %0 is out of range of flags in enumeration type %1">,
   InGroup<FlagEnum>;
-  
+def err_unknown_enumerator : Error<"%0 is not an enumerator of %1">;
 def warn_illegal_constant_array_size : Extension<
   "size of static array must be an integer constant expression">;
 def err_vm_decl_in_file_scope : Error<
   "variably modified type declaration not allowed at file scope">;
 def err_vm_decl_has_extern_linkage : Error<
   "variably modified type declaration cannot have 'extern' linkage">;
 def err_typecheck_field_variable_size : Error<
   "fields must have a constant size: 'variable length array in structure' "

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.
+    IdentifierInfo *NextName; //> If set, it follows '::' after current name.
+    SourceLocation NextIdLoc; //> Location of the next identifier.
+    bool IsLast; //> True if current name is the last in nested-name-spec.
+
+    bool CorrectionToColonRequested; //> Replacement '::' -> ':' recovers error.
+    bool IgnoreName; //> Ignore current name.
+
+    explicit NameSpecRecoveryInfo(bool CorrectToColon)
+      : CorrectToColon(CorrectToColon), NextName(nullptr), IsLast(false),
+        CorrectionToColonRequested(false), IgnoreName(false) { }
+  };
+
   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 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);
+      // To help error recovery, determine, if the current name is the final
+      // nested name component.
+      if (Tok.is(tok::identifier)) {
+        RecovInfo.NextName = Tok.getIdentifierInfo();
+        RecovInfo.NextIdLoc = Tok.getLocation();
+        RecovInfo.IsLast = NextToken().isNot(tok::coloncolon);
+      }
       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.IgnoreName)
+          // Error must be already reported.
+          break;
         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 (!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>()) {
+      // The current identifier does not represent an entity that may be used as
+      // a nested name spec, it nevertheless resolves to a declaration.  If the
+      // declaration is an enum, probably it is used as a scope for its
+      // enumerator. In Microsoft mode this is allowed usage.
+      bool DiagnosticNotIssued = true;
+      if (EnumDecl *ED = dyn_cast<EnumDecl>(TD)) {
+        assert(!ED->isScoped() && "Scoped enum must be already handled.");
+        if (getLangOpts().MicrosoftExt)
+          Diag(IdentifierLoc, diag::ext_ms_enum_in_nested_name_spec);
+        if (RecovInfo) {
+          if (RecovInfo->NextName &&
+              (getLangOpts().MicrosoftExt || RecovInfo->IsLast)) {
+            IdentifierInfo *NextId = RecovInfo->NextName;
+            for (auto Enumerator : ED->enumerators()) {
+              if (Enumerator->getName().equals(NextId->getName())) {
+                // We can recover parsing by throwing away enumeration name.
+                if (!getLangOpts().MicrosoftExt) {
+                  Diag(IdentifierLoc, diag::err_nested_name_spec_is_enum) <<
+                      FixItHint::CreateRemoval(SourceRange(IdentifierLoc, CCLoc));
+                }
+                DiagnosticNotIssued = false;
+                break;
+              }
+            }
+            if (DiagnosticNotIssued && getLangOpts().MicrosoftExt) {
+              // The current identifier represents enumeration, in Microsoft
+              // mode it must be followed by an enumerator of the respective
+              // enumeration.
+              Diag(RecovInfo->NextIdLoc, diag::err_unknown_enumerator)
+                  << NextId << ED;
+              DiagnosticNotIssued = false;
+            }
+          }
+        }
+        if (getLangOpts().MicrosoftExt && DiagnosticNotIssued)
+          return false;
+      }
+      if (DiagnosticNotIssued)
         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/ms-enum.cpp

+// RUN: %clang_cc1 %s -fsyntax-only -verify -fms-extensions
+
+namespace ns {
+  enum E1 {
+    V1
+  };
+}
+
+int x1 = ns::E1::V1;
+int x2 = ns::E1::V1::vvv; // expected -error{{'V1' is not a class, namespace, or scoped enumeration}}
+int x3 = ns::E1::X; // expected-error{{'X' is not an enumerator of 'E1'}}
+
+enum E2 {
+  V2
+};
+
+int x4 = E2::V2;
+int x5 = E2::X2; // expected-error{{'X2' is not an enumerator of 'E2'}}

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 cannot be specified as a 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 cannot be specified as a nested name specifier}}
+  int x2 = ns::an_enumeration::ENUMERATOR::vvv; // expected-error{{'PR16951::ns::an_enumeration' is not a class, namespace, or scoped enumeration}}
+  int x3 = ns::an_enumeration::X; // expected-error{{'PR16951::ns::an_enumeration' is not a class, namespace, or scoped enumeration}}
+
+  enum enumerator_2 {
+    ENUMERATOR_2
+  };
+
+  int x4 = enumerator_2::ENUMERATOR_2; // expected-error{{unscoped enumeration name cannot be specified as a nested name specifier}}
+  int x5 = enumerator_2::X2; // expected-error{{'PR16951::enumerator_2' is not a class, namespace, or scoped enumeration}}
+}