Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -16522,52 +16522,114 @@ // we find one that is. S = getNonFieldDeclScope(S); - // Verify that there isn't already something declared with this name in this - // scope. + // Verify that there isn't already a conflicting declaration with this name + // in this scope. LookupResult R(*this, Id, IdLoc, LookupOrdinaryName, ForVisibleRedeclaration); LookupName(R, S); - NamedDecl *PrevDecl = R.getAsSingle(); + NamedDecl *PrevDecl = nullptr; + switch (R.getResultKind()) { + case LookupResult::Found: + case LookupResult::FoundUnresolvedValue: + case LookupResult::FoundOverloaded: + // LookupResult::getAsSingle should not be used here for two reasons: + // - it looks through UsingShadowDecls (among other things). Using it + // instead would cause us to miss using-declarations. + // - it behaves as no declaration was found when the lookup result + // is not LookupResult::Found. This would cause us to miss + // UnresolvedUsingValueDecls and sets of overloaded functions. + PrevDecl = *R.begin(); + break; + + case LookupResult::NotFound: + case LookupResult::NotFoundInCurrentInstantiation: + break; + case LookupResult::Ambiguous: + // We don't have to care about an ambiguous lookup result since + // none of the cases in LookupResult::AmbiguityKind are relevant here. + break; + llvm_unreachable("Unexpected LookupResultKind"); + } + + // Check for shadowed template parameters. if (PrevDecl && PrevDecl->isTemplateParameter()) { - // Maybe we will complain about the shadowed template parameter. DiagnoseTemplateParameterShadow(IdLoc, PrevDecl); // Just pretend that we didn't see the previous declaration. PrevDecl = nullptr; } + // Look through using-declarations and namespace aliases. + NamedDecl *UnderlyingPrevDecl = + PrevDecl ? PrevDecl->getUnderlyingDecl() : nullptr; + assert((!PrevDecl || (PrevDecl && UnderlyingPrevDecl)) && + "A previous declaration was found but no underlying declaration ?"); + + // Check for a previous declaration which conflicts with the enumerator. + if (PrevDecl) { + // When in C++, we may get a TagDecl with the same name; in this case the + // enum constant will 'hide' the tag. + assert((getLangOpts().CPlusPlus || !isa(UnderlyingPrevDecl)) && + "Received TagDecl when not in C++!"); + // An enumerator can only hide the name of a class or enumeration that is + // not a typedef name ([basic.scope.declarative]p4). Additionally: + // + // 1. C++17 [namespace.udecl]p20: + // If a using-declarator uses the keyword typename and specifies a + // dependent name (17.6.2), the name introduced by the + // using-declaration is treated as a typedef-name + // + // Therefore we can just reject UnresolvedUsingTypenameDecls, even + // though they might resolve to a class or enumeration type during + // instantiation. + // + // 2. C++17 [namespace.udecl]p15 means that we need to look through + // using-declarations to determine if a declaration with the same + // name is allowed. + if (!isa(UnderlyingPrevDecl) && + isDeclInScope(PrevDecl, CurContext, S)) { + // If the enumerator conflicts with a (non-dependent) using-declaration, + // give more info about the conflict instead of a generic error. + if (auto *PrevDeclShadow = dyn_cast(PrevDecl)) { + Diag(IdLoc, diag::err_using_decl_conflict_reverse); + Diag(UnderlyingPrevDecl->getLocation(), diag::note_using_decl_target); + Diag(PrevDeclShadow->getUsingDecl()->getLocation(), + diag::note_using_decl) + << 0; + } else { + + if (isa(PrevDecl)) + Diag(IdLoc, diag::err_redefinition_of_enumerator) << Id; + + else + Diag(IdLoc, diag::err_redefinition) << Id; + + // FIXME: Don't use the term "definition" for a declaration + // which is not actually a definition (here and in general). + notePreviousDefinition(PrevDecl, IdLoc); + } + + return nullptr; + } + } + // C++ [class.mem]p15: - // If T is the name of a class, then each of the following shall have a name - // different from T: - // - every enumerator of every member of class T that is an unscoped - // enumerated type + // If T is the name of a class, then each of the following shall have a + // name different from T: + // - every enumerator of every member of class T that is an unscoped + // enumerated type if (getLangOpts().CPlusPlus && !TheEnumDecl->isScoped()) DiagnoseClassNameShadow(TheEnumDecl->getDeclContext(), DeclarationNameInfo(Id, IdLoc)); EnumConstantDecl *New = - CheckEnumConstant(TheEnumDecl, LastEnumConst, IdLoc, Id, Val); + CheckEnumConstant(TheEnumDecl, LastEnumConst, IdLoc, Id, Val); if (!New) return nullptr; - if (PrevDecl) { - if (!TheEnumDecl->isScoped() && isa(PrevDecl)) { - // Check for other kinds of shadowing not already handled. - CheckShadow(New, PrevDecl, R); - } - - // When in C++, we may get a TagDecl with the same name; in this case the - // enum constant will 'hide' the tag. - assert((getLangOpts().CPlusPlus || !isa(PrevDecl)) && - "Received TagDecl when not in C++!"); - if (!isa(PrevDecl) && isDeclInScope(PrevDecl, CurContext, S)) { - if (isa(PrevDecl)) - Diag(IdLoc, diag::err_redefinition_of_enumerator) << Id; - else - Diag(IdLoc, diag::err_redefinition) << Id; - notePreviousDefinition(PrevDecl, IdLoc); - return nullptr; - } - } + // Check for other kinds of shadowing not already handled. + if (PrevDecl && isa(UnderlyingPrevDecl) && + !TheEnumDecl->isScoped()) + CheckShadow(New, UnderlyingPrevDecl, R); // Process attributes. ProcessDeclAttributeList(S, New, Attrs); Index: test/CXX/dcl.dcl/dcl.enum/p11-enumerator_scope.cpp =================================================================== --- test/CXX/dcl.dcl/dcl.enum/p11-enumerator_scope.cpp +++ test/CXX/dcl.dcl/dcl.enum/p11-enumerator_scope.cpp @@ -6,36 +6,29 @@ // enumerator is declared in the scope of the enumeration. These names obey // the scope rules defined for all names in 6.3 and 6.4. -// FIXME: Actually implement the redeclaration lookup properly. // FIXME: Improve the wording of the error messages (definition vs declaration). -// FIXME: Only emit the Wshadow warning when the declaration of the -// enumerator does not conflict with another declaration. -// We also test for -Wshadow since the functionality is closely related, -// and we don't want to mess up Wshadow by fixing the redeclaration lookup. +// We also test for -Wshadow since the functionality is closely related. struct S_function { void f(); // expected-note 2{{previous definition}} - // FIXME: Don't emit shadow-note@-1 2{{previous declaration}} enum { f }; // expected-error {{redefinition of 'f'}} - // FIXME: Don't emit shadow-warning@-1 {{declaration shadows}} enum E1 { f }; // expected-error {{redefinition of 'f'}} - // FIXME: Don't emit shadow-warning@-1 {{declaration shadows}} enum class E2 { f }; // ok }; struct S_overloaded_function { void f(); - void f(int); - enum { f }; // FIXME: Reject. - enum E1 { f }; // FIXME: Reject. + void f(int); // expected-note 2{{previous definition}} + enum { f }; // expected-error {{redefinition of 'f'}} + enum E1 { f }; // expected-error {{redefinition of 'f'}} enum class E2 { f }; // ok }; struct S_function_template { - template void f(); - enum { f }; // FIXME: Reject. - enum E1 { f }; // FIXME: Reject. + template void f(); // expected-note 2{{previous definition}} + enum { f }; // expected-error {{redefinition of 'f'}} + enum E1 { f }; // expected-error {{redefinition of 'f'}} enum class E2 { f }; // ok }; @@ -51,12 +44,9 @@ struct S_member { struct f; int f; // expected-note {{previous definition}} - // FIXME: Don't emit shadow-note@-1 {{previous declaration}} static int g; // expected-note {{previous definition}} - // FIXME: Don't emit shadow-note@-1 {{previous declaration}} enum { f, g }; // expected-error {{redefinition of 'f'}} // expected-error@-1 {{redefinition of 'g'}} - // FIXME: Don't emit shadow-warning@-2 2{{declaration shadows}} }; namespace S_out_of_line_definition { @@ -77,17 +67,16 @@ namespace S_using_decl { struct Base { - int f; // FIXME: Don't emit shadow-note {{previous declaration}} + int f; // expected-note {{target of using declaration}} int g; struct s; - typedef struct s t; + typedef struct s t; // expected-note {{target of using declaration}} }; struct OtherBase {}; struct Der : Base, OtherBase { - using Base::f; - enum { f }; // FIXME: Reject. - // FIXME: Don't emit shadow-warning@-1 {{declaration shadows}} + using Base::f; // expected-note {{using declaration}} + enum { f }; // expected-error {{declaration conflicts with target of using declaration already in scope}} enum { g }; // ok using OtherBase::OtherBase; @@ -96,8 +85,8 @@ using Base::s; enum { s }; // ok - using Base::t; - enum { t }; // FIXME: Reject. + using Base::t; // expected-note {{using declaration}} + enum { t }; // expected-error {{declaration conflicts with target of using declaration already in scope}} }; } // namespace S_using_decl @@ -111,8 +100,8 @@ }; template struct Der : Base { - using Base::t; - enum { t }; // FIXME: Reject. + using Base::t; // expected-note {{previous definition}} + enum { t }; // expected-error {{redefinition of 't'}} // [namespace.udecl]p20: // If a using-declarator uses the keyword typename and specifies a // dependent name (17.6.2), the name introduced by the using-declaration @@ -131,26 +120,23 @@ namespace N_function { void f(); // expected-note 2{{previous definition}} - // FIXME: Don't emit shadow-note@-1 2{{previous declaration}} enum { f }; // expected-error {{redefinition of 'f'}} - // FIXME: Don't emit shadow-warning@-1 {{declaration shadows}} enum E1 { f }; // expected-error {{redefinition of 'f'}} - // FIXME: Don't emit shadow-warning@-1 {{declaration shadows}} enum class E2 { f }; // ok } namespace N_overloaded_function { void f(); - void f(int); - enum { f }; // FIXME: Reject. - enum E1 { f }; // FIXME: Reject. + void f(int); // expected-note 2{{previous definition}} + enum { f }; // expected-error {{redefinition of 'f'}} + enum E1 { f }; // expected-error {{redefinition of 'f'}} enum class E2 { f }; // ok } namespace N_function_template { - template void f(); - enum { f }; // FIXME: Reject. - enum E1 { f }; // FIXME: Reject. + template void f(); // expected-note 2{{previous definition}} + enum { f }; // expected-error {{redefinition of 'f'}} + enum E1 { f }; // expected-error {{redefinition of 'f'}} enum class E2 { f }; // ok } @@ -168,40 +154,36 @@ namespace N_variable { struct f; int f; // expected-note {{previous definition}} - // FIXME: Don't emit shadow-note@-1 {{previous declaration}} static int g; // expected-note {{previous definition}} - // FIXME: Don't emit shadow-note@-1 {{previous declaration}} enum { f, g }; // expected-error {{redefinition of 'f'}} // expected-error@-1 {{redefinition of 'g'}} - // FIXME: Don't emit shadow-warning@-2 2{{declaration shadows}} } namespace N_using_decl_aux { - int f; // shadow-note {{previous declaration}} + int f; // expected-note {{target of using declaration}} struct s; - typedef struct s t; + typedef struct s t; // expected-note {{target of using declaration}} } namespace N_using_decl { - using N_using_decl_aux::f; - enum { f }; // FIXME: Reject. - // shadow-warning@-1 {{declaration shadows a variable in namespace 'N_using_decl_aux'}} + using N_using_decl_aux::f; // expected-note {{using declaration}} + enum { f }; // expected-error {{declaration conflicts with target of using declaration already in scope}} using N_using_decl_aux::s; enum { s }; // ok - using N_using_decl_aux::t; - enum { t }; // FIXME: Reject. + using N_using_decl_aux::t; // expected-note {{using declaration}} + enum { t }; // expected-error {{declaration conflicts with target of using declaration already in scope}} } namespace N_conflicting_namespace_alias { - namespace Q { int i; } // FIXME: expected-note 2{{previous definition}} + namespace Q { int i; } namespace M { namespace i = Q; } using namespace M; - enum { i }; // FIXME: This is fine. expected-error {{redefinition of 'i'}} + enum { i }; // ok int j = i::i; // ok - namespace k = M::i; + namespace k = M::i; // expected-note {{previous definition}} enum { k }; // expected-error {{redefinition of 'k'}} } @@ -215,13 +197,10 @@ } namespace N_enumerator_redefinition { - namespace N { enum { e }; } // FIXME: shadow-note {{previous declaration}} - using N::e; - enum { e }; // FIXME: Reject. - // FIXME: Don't emit shadow-warning@-1 {{declaration shadows}} + namespace N { enum { e }; } // expected-note {{target of using declaration}} + using N::e; // expected-note {{using declaration}} + enum { e }; // expected-error {{declaration conflicts with target of using declaration already in scope}} enum { f }; // expected-note {{previous definition}} - // FIXME: shadow-note@-1 {{previous declaration}} enum { f }; // expected-error {{redefinition of enumerator 'f'}} - // FIXME: Don't emit shadow-warning@-1 {{declaration shadows}} }