riccibruno updated this revision to Diff 197225.
riccibruno retitled this revision from "[Sema] Fix the lookup for a declaration
conflicting with an enumerator (bogus use of LookupResult::getAsSingle)" to
"[Sema] Fix the lookup for a declaration conflicting with an enumerator".
riccibruno edited the summary of this revision.
riccibruno added a reviewer: rjmccall.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60956/new/
https://reviews.llvm.org/D60956
Files:
lib/Sema/SemaDecl.cpp
test/CXX/dcl.dcl/dcl.enum/p11-enumerator_scope.cpp
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 <typename> void f();
- enum { f }; // FIXME: Reject.
- enum E1 { f }; // FIXME: Reject.
+ template <typename> 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 <typename T> struct Der : Base<T> {
- using Base<T>::t;
- enum { t }; // FIXME: Reject.
+ using Base<T>::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 <typename> void f();
- enum { f }; // FIXME: Reject.
- enum E1 { f }; // FIXME: Reject.
+ template <typename> 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}}
}
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>();
+ 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<TagDecl>(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<TagDecl>(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<UsingShadowDecl>(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<EnumConstantDecl>(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<ValueDecl>(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<TagDecl>(PrevDecl)) &&
- "Received TagDecl when not in C++!");
- if (!isa<TagDecl>(PrevDecl) && isDeclInScope(PrevDecl, CurContext, S)) {
- if (isa<EnumConstantDecl>(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<ValueDecl>(UnderlyingPrevDecl) &&
+ !TheEnumDecl->isScoped())
+ CheckShadow(New, UnderlyingPrevDecl, R);
// Process attributes.
ProcessDeclAttributeList(S, New, Attrs);
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
