[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator
riccibruno added a comment. Ping :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60956/new/ https://reviews.llvm.org/D60956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator
riccibruno updated this revision to Diff 198629.
riccibruno marked 4 inline comments as done.
riccibruno added a comment.
Address Aaron's 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,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'}}
-
[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator
aaron.ballman added a comment.
I found a few nits, but generally think this LG. However, I think @rsmith
should sign off on it just in case I've misinterpreted something along the way.
Comment at: lib/Sema/SemaDecl.cpp:16537
+// 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
behaves as no -> behaves as if no
Comment at: lib/Sema/SemaDecl.cpp:16551
+llvm_unreachable("Unexpected LookupResultKind");
+ }
Missing a `default` label in front of this?
Comment at: lib/Sema/SemaDecl.cpp:16599
+ } else {
+
+if (isa(PrevDecl))
Spurious newline
Comment at: lib/Sema/SemaDecl.cpp:16602
+ Diag(IdLoc, diag::err_redefinition_of_enumerator) << Id;
+
+else
Spurious newline
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60956/new/
https://reviews.llvm.org/D60956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator
riccibruno marked 3 inline comments as done.
riccibruno added inline comments.
Comment at: test/CXX/dcl.dcl/dcl.enum/p11-enumerator_scope.cpp:107
enum { typedef_type };// expected-error {{redefinition of
'typedef_type'}}
};
riccibruno wrote:
> Note also that this is just my interpretation, and I am not entirely
> confident that this is correct.
>
> As a data point GCC rejects `enum { t }` in the template and accepts the
> other two enumerators, and then in the instantiation rejects `enum { t }` and
> `enum { typedef_type }` but accepts `enum { type }` (presumably because it
> delays the check for the hiding rules to the instantiation).
I have checked and I believe that this is exactly what CWG 11 says.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60956/new/
https://reviews.llvm.org/D60956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator
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 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 };
[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator (bogus use of LookupResult::getAsSingle)
riccibruno marked an inline comment as done.
riccibruno added inline comments.
Comment at: test/CXX/dcl.dcl/dcl.enum/p11-enumerator_scope.cpp:107
enum { typedef_type };// expected-error {{redefinition of
'typedef_type'}}
};
Note also that this is just my interpretation, and I am not entirely confident
that this is correct.
As a data point GCC rejects `enum { t }` in the template and accepts the other
two enumerators, and then in the instantiation rejects `enum { t }` and `enum {
typedef_type }` but accepts `enum { type }` (presumably because it delays the
check for the hiding rules to the instantiation).
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60956/new/
https://reviews.llvm.org/D60956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator (bogus use of LookupResult::getAsSingle)
riccibruno added a comment. > 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` is 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. I meant in the above "[...] `PrevDecl` was the `NamespaceDecl` for `Q`, and not the `NamespaceAliasDecl` for `M::i`" Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60956/new/ https://reviews.llvm.org/D60956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator (bogus use of LookupResult::getAsSingle)
riccibruno marked an inline comment as done.
riccibruno added inline comments.
Comment at: lib/Sema/SemaDecl.cpp:16607
+ // Check for other kinds of shadowing not already handled.
+ if (PrevDecl && isa(PrevDecl->getUnderlyingDecl()) &&
+ !TheEnumDecl->isScoped())
aaron.ballman wrote:
> riccibruno wrote:
> > aaron.ballman wrote:
> > > Is the change to `PrevDecl->getUnderlyingDecl()` intended here?
> > Yes it is. Previously it was done inside `LookupResult::getAsSingle`.
> > However with this patch `PrevDecl` at this point can be a `UsingShadowDecl`
> > for a given using-declaration. We need to look for the underlying
> > declaration since this is what `CheckShadow` expects.
> But when it's not a `UsingShadowDecl`, will the behavior now be incorrect?
> e.g., if it was a `NamespaceAliasDecl`, won't this check whether you are
> shadowing the aliased namespace as opposed to the alias name itself? Might be
> worth some tests.
Do you mean in an example like the following ?
```
namespace Q {}
namespace M { namespace i = Q; }
using namespace M;
enum { i };
```
This is example is mistakenly rejected (I think!) because of the fact that
currently `getAsSingle` will look through the `NamespaceAliasDecl` for `i`, and
find the `NamespaceDecl` for `Q`. There are currently no shadow warning for
such an example.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60956/new/
https://reviews.llvm.org/D60956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator (bogus use of LookupResult::getAsSingle)
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 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
};
@@ -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 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
@@ -122,32 +111,27 @@
// Conflict with another declaration at namespace scope
namespace N_function {
void f(); // expected-note 2{{previous definition}}
[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator (bogus use of LookupResult::getAsSingle)
aaron.ballman added inline comments. Comment at: lib/Sema/SemaDecl.cpp:16607 + // Check for other kinds of shadowing not already handled. + if (PrevDecl && isa(PrevDecl->getUnderlyingDecl()) && + !TheEnumDecl->isScoped()) riccibruno wrote: > aaron.ballman wrote: > > Is the change to `PrevDecl->getUnderlyingDecl()` intended here? > Yes it is. Previously it was done inside `LookupResult::getAsSingle`. However > with this patch `PrevDecl` at this point can be a `UsingShadowDecl` for a > given using-declaration. We need to look for the underlying declaration since > this is what `CheckShadow` expects. But when it's not a `UsingShadowDecl`, will the behavior now be incorrect? e.g., if it was a `NamespaceAliasDecl`, won't this check whether you are shadowing the aliased namespace as opposed to the alias name itself? Might be worth some tests. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60956/new/ https://reviews.llvm.org/D60956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator (bogus use of LookupResult::getAsSingle)
riccibruno marked an inline comment as done. riccibruno added inline comments. Comment at: lib/Sema/SemaDecl.cpp:16607 + // Check for other kinds of shadowing not already handled. + if (PrevDecl && isa(PrevDecl->getUnderlyingDecl()) && + !TheEnumDecl->isScoped()) aaron.ballman wrote: > Is the change to `PrevDecl->getUnderlyingDecl()` intended here? Yes it is. Previously it was done inside `LookupResult::getAsSingle`. However with this patch `PrevDecl` at this point can be a `UsingShadowDecl` for a given using-declaration. We need to look for the underlying declaration since this is what `CheckShadow` expects. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60956/new/ https://reviews.llvm.org/D60956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator (bogus use of LookupResult::getAsSingle)
aaron.ballman added inline comments. Comment at: lib/Sema/SemaDecl.cpp:16607 + // Check for other kinds of shadowing not already handled. + if (PrevDecl && isa(PrevDecl->getUnderlyingDecl()) && + !TheEnumDecl->isScoped()) Is the change to `PrevDecl->getUnderlyingDecl()` intended here? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60956/new/ https://reviews.llvm.org/D60956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator (bogus use of LookupResult::getAsSingle)
riccibruno created this revision.
riccibruno added reviewers: aaron.ballman, rnk, rsmith.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.
Currently the lookup for a declaration conflicting with an enumerator is pretty
broken, because of the use of `LookupResult::getAsSingle()` (see the
tests for examples).
This patch fixes this by removing the bogus use of `getAsSingle()`.
While we are at it, also do the check for a conflicting declaration before
calling `CheckShadow` (which implements `Wshadow`). This avoids redundant
`Wshadow` warnings where we already have an error.
This is not the only place where `getAsSingle` is mistakenly used, but this is
better addressed in a later patch.
Repository:
rC Clang
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 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
};
@@ -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 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
@@ -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
