riccibruno updated this revision to Diff 196264.
riccibruno added a comment.
- Added a test (see `N_shadow)` for the behavior of `Wshadow`. This test showed
that I forgot to change `CheckShadow(New, PrevDecl, R);` to `CheckShadow(New,
PrevDecl->getUnderlyingDecl(), R);` to match change in the condition of the if
statement.
- Added a test which exposes a new problem that this patch incidentally solves
(see `N_conflicting_namespace_alias`). Because of the using directive `using
namespace M;`, the namespace alias `i` for the namespace `Q` if found in the
redeclaration lookup. Before this patch, since `getAsSingle` used internally
`getUnderlyingDecl()`, `PrevDecl` was for the `NamespaceDecl` for `Q`, and not
for the `VarDecl` for `Q::i`. Then the declaration for the enumerator `i` was
mistakenly rejected since `Q` is in the same scope.
- Clarified some comments.
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,37 +6,30 @@
// 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.
// Conflict with another declaration at class scope.
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
};
@@ -52,12 +45,9 @@
struct S_data_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 {
@@ -78,14 +68,13 @@
namespace S_using_decl {
struct Base {
- int f; // FIXME: Don't emit shadow-note {{previous declaration}}
+ int f;
int g;
};
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 {{previous definition}}
+ enum { f }; // expected-error {{redefinition of 'f'}}
enum { g }; // ok
using OtherBase::OtherBase;
@@ -103,8 +92,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
@@ -122,32 +111,27 @@
// Conflict with another declaration at namespace scope
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
extern void g(); // expected-note {{previous definition}}
- // FIXME: Don't emit shadow-note@-1 {{previous declaration}}
enum { g }; // expected-error {{redefinition of 'g'}}
- // FIXME: Don't emit shadow-warning@-1 {{declaration shadows}}
enum class E3 { g }; // 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
}
@@ -165,27 +149,23 @@
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}}
+namespace N_using_decl_aux { int f; }
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 {{previous definition}}
+ enum { f }; // expected-error {{redefinition of 'f'}}
}
namespace N_conflicting_namespace_alias {
- namespace Q { int i; } // FIXME: expected-note {{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
}
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -16522,53 +16522,93 @@
// 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;
}
- // 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 (getLangOpts().CPlusPlus && !TheEnumDecl->isScoped())
- DiagnoseClassNameShadow(TheEnumDecl->getDeclContext(),
- DeclarationNameInfo(Id, IdLoc));
-
- EnumConstantDecl *New =
- CheckEnumConstant(TheEnumDecl, LastEnumConst, IdLoc, Id, Val);
- if (!New)
- return nullptr;
-
+ // Check for a previous declaration which conflicts with the enumerator.
+ // [dcl.enum]p11
+ // Each enum-name and each unscoped enumerator is declared in the scope
+ // that immediately contains the enum-specifier. Each scoped enumerator
+ // is declared in the scope of the enumeration. These names obey the scope
+ // rules defined for all names in [basic.scope] and [basic.lookup].
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++!");
+ // An enumerator can only hide the name of a class or enumeration that is
+ // not a typedef name ([basic.scope.declarative]p4). Additionally:
+ // [namespace.udir]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.
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;
+
+ // 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 (getLangOpts().CPlusPlus && !TheEnumDecl->isScoped())
+ DiagnoseClassNameShadow(TheEnumDecl->getDeclContext(),
+ DeclarationNameInfo(Id, IdLoc));
+
+ EnumConstantDecl *New =
+ CheckEnumConstant(TheEnumDecl, LastEnumConst, IdLoc, Id, Val);
+ if (!New)
+ return nullptr;
+
+ // Check for other kinds of shadowing not already handled.
+ if (PrevDecl && isa<ValueDecl>(PrevDecl->getUnderlyingDecl()) &&
+ !TheEnumDecl->isScoped())
+ CheckShadow(New, PrevDecl->getUnderlyingDecl(), R);
+
// Process attributes.
ProcessDeclAttributeList(S, New, Attrs);
AddPragmaAttributes(S, New);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
