[clang] [Clang] skip shadow warnings for enum constants in distinct class scopes (PR #115656)
Oleksandr T. via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 13 06:36:18 PST 2024
https://github.com/a-tarasyuk updated https://github.com/llvm/llvm-project/pull/115656
>From e13774913ecba7d66652f10f9a3468a55acb3b42 Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Sun, 10 Nov 2024 15:11:39 +0200
Subject: [PATCH 1/2] [Clang] skip shadow warnings for enum constants in
distinct class scopes
---
clang/docs/ReleaseNotes.rst | 2 ++
clang/include/clang/Sema/Sema.h | 2 +-
clang/lib/Sema/SemaDecl.cpp | 15 ++++++++++-----
clang/lib/Sema/SemaDeclCXX.cpp | 2 +-
clang/test/SemaCXX/warn-shadow.cpp | 14 ++++++++++++++
5 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c3424e0e6f34c9..f8005c57ea865c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -527,6 +527,8 @@ Improvements to Clang's diagnostics
- Clang now diagnoses ``[[deprecated]]`` attribute usage on local variables (#GH90073).
+- Clang now omits shadow warnings for enum constants in separate class scopes (#GH62588).
+
Improvements to Clang's time-trace
----------------------------------
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index fad446a05e782f..8e8097e86764c6 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3453,7 +3453,7 @@ class Sema final : public SemaBase {
///
/// \param ShadowedDecl the declaration that is shadowed by the given variable
/// \param R the lookup of the name
- void CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
+ void CheckShadow(Scope *S, NamedDecl *D, NamedDecl *ShadowedDecl,
const LookupResult &R);
/// Check -Wshadow without the advantage of a previous lookup.
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 61c29e320d5c73..7bf7a19de1c6a6 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6732,7 +6732,7 @@ Sema::ActOnTypedefNameDecl(Scope *S, DeclContext *DC, TypedefNameDecl *NewTD,
}
if (ShadowedDecl && !Redeclaration)
- CheckShadow(NewTD, ShadowedDecl, Previous);
+ CheckShadow(S, NewTD, ShadowedDecl, Previous);
// If this is the C FILE type, notify the AST context.
if (IdentifierInfo *II = NewTD->getIdentifier())
@@ -8092,7 +8092,7 @@ NamedDecl *Sema::ActOnVariableDeclarator(
// Diagnose shadowed variables iff this isn't a redeclaration.
if (!IsPlaceholderVariable && ShadowedDecl && !D.isRedeclaration())
- CheckShadow(NewVD, ShadowedDecl, Previous);
+ CheckShadow(S, NewVD, ShadowedDecl, Previous);
ProcessPragmaWeak(S, NewVD);
@@ -8237,7 +8237,7 @@ NamedDecl *Sema::getShadowedDeclaration(const BindingDecl *D,
: nullptr;
}
-void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
+void Sema::CheckShadow(Scope *S, NamedDecl *D, NamedDecl *ShadowedDecl,
const LookupResult &R) {
DeclContext *NewDC = D->getDeclContext();
@@ -8341,6 +8341,11 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
// shadowing context, but that's just a false negative.
}
+ // Skip shadowing check if we're in a class scope, dealing with an enum
+ // constant in a different context.
+ if (S->isClassScope() && isa<EnumConstantDecl>(D) &&
+ !OldDC->Equals(NewDC->getRedeclContext()))
+ return;
DeclarationName Name = R.getLookupName();
@@ -8389,7 +8394,7 @@ void Sema::CheckShadow(Scope *S, VarDecl *D) {
RedeclarationKind::ForVisibleRedeclaration);
LookupName(R, S);
if (NamedDecl *ShadowedDecl = getShadowedDeclaration(D, R))
- CheckShadow(D, ShadowedDecl, R);
+ CheckShadow(S, D, ShadowedDecl, R);
}
/// Check if 'E', which is an expression that is about to be modified, refers
@@ -19736,7 +19741,7 @@ Decl *Sema::ActOnEnumConstant(Scope *S, Decl *theEnumDecl, Decl *lastEnumConst,
if (PrevDecl) {
if (!TheEnumDecl->isScoped() && isa<ValueDecl>(PrevDecl)) {
// Check for other kinds of shadowing not already handled.
- CheckShadow(New, PrevDecl, R);
+ CheckShadow(S, New, PrevDecl, R);
}
// When in C++, we may get a TagDecl with the same name; in this case the
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 8d76a35b2d2557..ad8df19485d9da 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -924,7 +924,7 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator &D,
Diag(Old->getLocation(), diag::note_previous_definition);
}
} else if (ShadowedDecl && !D.isRedeclaration()) {
- CheckShadow(BD, ShadowedDecl, Previous);
+ CheckShadow(S, BD, ShadowedDecl, Previous);
}
PushOnScopeChains(BD, S, true);
Bindings.push_back(BD);
diff --git a/clang/test/SemaCXX/warn-shadow.cpp b/clang/test/SemaCXX/warn-shadow.cpp
index 2969bd39fed41e..98a235a73c7e52 100644
--- a/clang/test/SemaCXX/warn-shadow.cpp
+++ b/clang/test/SemaCXX/warn-shadow.cpp
@@ -307,3 +307,17 @@ void test4() {
}
}; // namespace structured_binding_tests
+
+namespace GH62588 {
+class Outer {
+public:
+ char *foo(); // expected-note {{previous declaration is here}} \
+ // expected-note {{previous definition is here}}
+ enum Outer_E { foo }; // expected-error {{redefinition of 'foo'}} \
+ // expected-warning {{declaration shadows a static data member of 'GH62588::Outer'}}
+ class Inner {
+ public:
+ enum Inner_E { foo }; // ok
+ };
+};
+} // namespace GH62588
>From b448063ae9329511bf30eddb8f719ee33c10cd4d Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Tue, 12 Nov 2024 15:51:24 +0200
Subject: [PATCH 2/2] remove redundant Scope parameter
---
clang/include/clang/Sema/Sema.h | 2 +-
clang/lib/Sema/SemaDecl.cpp | 26 +++++++++++++-------------
clang/lib/Sema/SemaDeclCXX.cpp | 2 +-
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 8e8097e86764c6..fad446a05e782f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3453,7 +3453,7 @@ class Sema final : public SemaBase {
///
/// \param ShadowedDecl the declaration that is shadowed by the given variable
/// \param R the lookup of the name
- void CheckShadow(Scope *S, NamedDecl *D, NamedDecl *ShadowedDecl,
+ void CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
const LookupResult &R);
/// Check -Wshadow without the advantage of a previous lookup.
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 7bf7a19de1c6a6..59bd21cc3184b4 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6732,7 +6732,7 @@ Sema::ActOnTypedefNameDecl(Scope *S, DeclContext *DC, TypedefNameDecl *NewTD,
}
if (ShadowedDecl && !Redeclaration)
- CheckShadow(S, NewTD, ShadowedDecl, Previous);
+ CheckShadow(NewTD, ShadowedDecl, Previous);
// If this is the C FILE type, notify the AST context.
if (IdentifierInfo *II = NewTD->getIdentifier())
@@ -8092,7 +8092,7 @@ NamedDecl *Sema::ActOnVariableDeclarator(
// Diagnose shadowed variables iff this isn't a redeclaration.
if (!IsPlaceholderVariable && ShadowedDecl && !D.isRedeclaration())
- CheckShadow(S, NewVD, ShadowedDecl, Previous);
+ CheckShadow(NewVD, ShadowedDecl, Previous);
ProcessPragmaWeak(S, NewVD);
@@ -8237,7 +8237,7 @@ NamedDecl *Sema::getShadowedDeclaration(const BindingDecl *D,
: nullptr;
}
-void Sema::CheckShadow(Scope *S, NamedDecl *D, NamedDecl *ShadowedDecl,
+void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
const LookupResult &R) {
DeclContext *NewDC = D->getDeclContext();
@@ -8328,9 +8328,15 @@ void Sema::CheckShadow(Scope *S, NamedDecl *D, NamedDecl *ShadowedDecl,
return;
// Only warn about certain kinds of shadowing for class members.
- if (NewDC && NewDC->isRecord()) {
+ if (NewDC) {
// In particular, don't warn about shadowing non-class members.
- if (!OldDC->isRecord())
+ if (NewDC->isRecord() && !OldDC->isRecord())
+ return;
+
+ // Skip shadowing check if we're in a class scope, dealing with an enum
+ // constant in a different context.
+ DeclContext *ReDC = NewDC->getRedeclContext();
+ if (ReDC->isRecord() && isa<EnumConstantDecl>(D) && !OldDC->Equals(ReDC))
return;
// TODO: should we warn about static data members shadowing
@@ -8341,12 +8347,6 @@ void Sema::CheckShadow(Scope *S, NamedDecl *D, NamedDecl *ShadowedDecl,
// shadowing context, but that's just a false negative.
}
- // Skip shadowing check if we're in a class scope, dealing with an enum
- // constant in a different context.
- if (S->isClassScope() && isa<EnumConstantDecl>(D) &&
- !OldDC->Equals(NewDC->getRedeclContext()))
- return;
-
DeclarationName Name = R.getLookupName();
// Emit warning and note.
@@ -8394,7 +8394,7 @@ void Sema::CheckShadow(Scope *S, VarDecl *D) {
RedeclarationKind::ForVisibleRedeclaration);
LookupName(R, S);
if (NamedDecl *ShadowedDecl = getShadowedDeclaration(D, R))
- CheckShadow(S, D, ShadowedDecl, R);
+ CheckShadow(D, ShadowedDecl, R);
}
/// Check if 'E', which is an expression that is about to be modified, refers
@@ -19741,7 +19741,7 @@ Decl *Sema::ActOnEnumConstant(Scope *S, Decl *theEnumDecl, Decl *lastEnumConst,
if (PrevDecl) {
if (!TheEnumDecl->isScoped() && isa<ValueDecl>(PrevDecl)) {
// Check for other kinds of shadowing not already handled.
- CheckShadow(S, New, PrevDecl, R);
+ CheckShadow(New, PrevDecl, R);
}
// When in C++, we may get a TagDecl with the same name; in this case the
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index ad8df19485d9da..8d76a35b2d2557 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -924,7 +924,7 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator &D,
Diag(Old->getLocation(), diag::note_previous_definition);
}
} else if (ShadowedDecl && !D.isRedeclaration()) {
- CheckShadow(S, BD, ShadowedDecl, Previous);
+ CheckShadow(BD, ShadowedDecl, Previous);
}
PushOnScopeChains(BD, S, true);
Bindings.push_back(BD);
More information about the cfe-commits
mailing list