r294401 - Sema: add warning for c++ member variable shadowing
Saleem Abdulrasool via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 7 19:30:14 PST 2017
Author: compnerd
Date: Tue Feb 7 21:30:13 2017
New Revision: 294401
URL: http://llvm.org/viewvc/llvm-project?rev=294401&view=rev
Log:
Sema: add warning for c++ member variable shadowing
Add a warning for shadowed variables across records. Referencing a
shadow'ed variable may not give the desired variable. Add an optional
warning for the shadowing.
Patch by James Sun!
Added:
cfe/trunk/test/CXX/class.derived/class.member.lookup/p10.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CXX/class.derived/class.member.lookup/p6.cpp
cfe/trunk/test/CXX/class.derived/class.member.lookup/p7.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=294401&r1=294400&r2=294401&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Feb 7 21:30:13 2017
@@ -355,6 +355,7 @@ def SemiBeforeMethodBody : DiagGroup<"se
def Sentinel : DiagGroup<"sentinel">;
def MissingMethodReturnType : DiagGroup<"missing-method-return-type">;
+def ShadowField : DiagGroup<"shadow-field">;
def ShadowFieldInConstructorModified : DiagGroup<"shadow-field-in-constructor-modified">;
def ShadowFieldInConstructor : DiagGroup<"shadow-field-in-constructor",
[ShadowFieldInConstructorModified]>;
@@ -366,7 +367,7 @@ def ShadowUncapturedLocal : DiagGroup<"s
def Shadow : DiagGroup<"shadow", [ShadowFieldInConstructorModified,
ShadowIvar]>;
def ShadowAll : DiagGroup<"shadow-all", [Shadow, ShadowFieldInConstructor,
- ShadowUncapturedLocal]>;
+ ShadowUncapturedLocal, ShadowField]>;
def Shorten64To32 : DiagGroup<"shorten-64-to-32">;
def : DiagGroup<"sign-promo">;
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=294401&r1=294400&r2=294401&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Feb 7 21:30:13 2017
@@ -8959,4 +8959,9 @@ def ext_warn_gnu_final : ExtWarn<
"__final is a GNU extension, consider using C++11 final">,
InGroup<GccCompat>;
+def warn_shadow_field :
+ Warning<"non-static data member '%0' of '%1' shadows member inherited from type '%2'">,
+ InGroup<ShadowField>, DefaultIgnore;
+def note_shadow_field : Note<"declared here">;
+
} // end of sema component.
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=294401&r1=294400&r2=294401&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Feb 7 21:30:13 2017
@@ -10074,6 +10074,11 @@ private:
void CheckBitFieldInitialization(SourceLocation InitLoc, FieldDecl *Field,
Expr *Init);
+ /// Check if there is a field shadowing.
+ void CheckShadowInheritedFields(const SourceLocation &Loc,
+ DeclarationName FieldName,
+ const CXXRecordDecl *RD);
+
/// \brief Check if the given expression contains 'break' or 'continue'
/// statement that produces control flow different from GCC.
void CheckBreakContinueBinding(Expr *E);
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=294401&r1=294400&r2=294401&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Feb 7 21:30:13 2017
@@ -2758,6 +2758,56 @@ static AttributeList *getMSPropertyAttr(
return nullptr;
}
+// Check if there is a field shadowing.
+void Sema::CheckShadowInheritedFields(const SourceLocation &Loc,
+ DeclarationName FieldName,
+ const CXXRecordDecl *RD) {
+ if (Diags.isIgnored(diag::warn_shadow_field, Loc))
+ return;
+
+ // To record a shadowed field in a base
+ std::map<CXXRecordDecl*, NamedDecl*> Bases;
+ auto FieldShadowed = [&](const CXXBaseSpecifier *Specifier,
+ CXXBasePath &Path) {
+ const auto Base = Specifier->getType()->getAsCXXRecordDecl();
+ // Record an ambiguous path directly
+ if (Bases.find(Base) != Bases.end())
+ return true;
+ for (const auto Field : Base->lookup(FieldName)) {
+ if ((isa<FieldDecl>(Field) || isa<IndirectFieldDecl>(Field)) &&
+ Field->getAccess() != AS_private) {
+ assert(Field->getAccess() != AS_none);
+ assert(Bases.find(Base) == Bases.end());
+ Bases[Base] = Field;
+ return true;
+ }
+ }
+ return false;
+ };
+
+ CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/true,
+ /*DetectVirtual=*/true);
+ if (!RD->lookupInBases(FieldShadowed, Paths))
+ return;
+
+ for (const auto &P : Paths) {
+ auto Base = P.back().Base->getType()->getAsCXXRecordDecl();
+ auto It = Bases.find(Base);
+ // Skip duplicated bases
+ if (It == Bases.end())
+ continue;
+ auto BaseField = It->second;
+ assert(BaseField->getAccess() != AS_private);
+ if (AS_none !=
+ CXXRecordDecl::MergeAccess(P.Access, BaseField->getAccess())) {
+ Diag(Loc, diag::warn_shadow_field)
+ << FieldName.getAsString() << RD->getName() << Base->getName();
+ Diag(BaseField->getLocation(), diag::note_shadow_field);
+ Bases.erase(It);
+ }
+ }
+}
+
/// ActOnCXXMemberDeclarator - This is invoked when a C++ class member
/// declarator is parsed. 'AS' is the access specifier, 'BW' specifies the
/// bitfield width if there is one, 'InitExpr' specifies the initializer if
@@ -2957,6 +3007,11 @@ Sema::ActOnCXXMemberDeclarator(Scope *S,
if (!Member)
return nullptr;
}
+
+ // Check for any possible shadowed member variables
+ if (const auto *RD = cast<CXXRecordDecl>(CurContext))
+ CheckShadowInheritedFields(Loc, Name, RD);
+
} else {
Member = HandleDeclarator(S, D, TemplateParameterLists);
if (!Member)
Added: cfe/trunk/test/CXX/class.derived/class.member.lookup/p10.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.derived/class.member.lookup/p10.cpp?rev=294401&view=auto
==============================================================================
--- cfe/trunk/test/CXX/class.derived/class.member.lookup/p10.cpp (added)
+++ cfe/trunk/test/CXX/class.derived/class.member.lookup/p10.cpp Tue Feb 7 21:30:13 2017
@@ -0,0 +1,114 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wshadow-all
+
+// Basic cases, ambiguous paths, and fields with different access
+class A {
+public:
+ int x; // expected-note 2{{declared here}}
+protected:
+ int y; // expected-note 2{{declared here}}
+private:
+ int z;
+};
+
+struct B : A {
+};
+
+struct C : A {
+};
+
+struct W {
+ int w; // expected-note {{declared here}}
+};
+
+struct U : W {
+};
+
+struct V : W {
+};
+
+class D {
+public:
+ char w; // expected-note {{declared here}}
+private:
+ char x;
+};
+
+// Check direct inheritance and multiple paths to the same base.
+class E : B, C, D, U, V
+{
+ unsigned x; // expected-warning {{non-static data member 'x' of 'E' shadows member inherited from type 'A'}}
+ char y; // expected-warning {{non-static data member 'y' of 'E' shadows member inherited from type 'A'}}
+ double z;
+ char w; // expected-warning {{non-static data member 'w' of 'E' shadows member inherited from type 'D'}} expected-warning {{non-static data member 'w' of 'E' shadows member inherited from type 'W'}}
+};
+
+// Virtual inheritance
+struct F : virtual A {
+};
+
+struct G : virtual A {
+};
+
+class H : F, G {
+ int x; // expected-warning {{non-static data member 'x' of 'H' shadows member inherited from type 'A'}}
+ int y; // expected-warning {{non-static data member 'y' of 'H' shadows member inherited from type 'A'}}
+ int z;
+};
+
+// Indirect inheritance
+struct I {
+ union {
+ int x; // expected-note {{declared here}}
+ int y;
+ };
+};
+
+struct J : I {
+ int x; // expected-warning {{non-static data member 'x' of 'J' shadows member inherited from type 'I'}}
+};
+
+// non-access paths
+class N : W {
+};
+
+struct K {
+ int y;
+};
+
+struct L : private K {
+};
+
+struct M : L {
+ int y;
+ int w;
+};
+
+// Multiple ambiguous paths with different accesses
+struct A1 {
+ int x; // expected-note {{declared here}}
+};
+
+class B1 : A1 {
+};
+
+struct B2 : A1 {
+};
+
+struct C1 : B1, B2 {
+};
+
+class D1 : C1 {
+};
+
+struct D2 : C1 {
+};
+
+class D3 : C1 {
+};
+
+struct E1 : D1, D2, D3{
+ int x; // expected-warning {{non-static data member 'x' of 'E1' shadows member inherited from type 'A1'}}
+};
+
+
+
Modified: cfe/trunk/test/CXX/class.derived/class.member.lookup/p6.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.derived/class.member.lookup/p6.cpp?rev=294401&r1=294400&r2=294401&view=diff
==============================================================================
--- cfe/trunk/test/CXX/class.derived/class.member.lookup/p6.cpp (original)
+++ cfe/trunk/test/CXX/class.derived/class.member.lookup/p6.cpp Tue Feb 7 21:30:13 2017
@@ -1,24 +1,24 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wshadow-field
class V {
public:
int f();
- int x;
+ int x; // expected-note {{declared here}}
};
class W {
public:
int g(); // expected-note{{member found by ambiguous name lookup}}
- int y; // expected-note{{member found by ambiguous name lookup}}
+ int y; // expected-note{{member found by ambiguous name lookup}} expected-note {{declared here}}
};
class B : public virtual V, public W
{
public:
int f();
- int x;
+ int x; // expected-warning {{non-static data member 'x' of 'B' shadows member inherited from type 'V'}}
int g(); // expected-note{{member found by ambiguous name lookup}}
- int y; // expected-note{{member found by ambiguous name lookup}}
+ int y; // expected-note{{member found by ambiguous name lookup}} expected-warning {{non-static data member 'y' of 'B' shadows member inherited from type 'W'}}
};
class C : public virtual V, public W { };
Modified: cfe/trunk/test/CXX/class.derived/class.member.lookup/p7.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.derived/class.member.lookup/p7.cpp?rev=294401&r1=294400&r2=294401&view=diff
==============================================================================
--- cfe/trunk/test/CXX/class.derived/class.member.lookup/p7.cpp (original)
+++ cfe/trunk/test/CXX/class.derived/class.member.lookup/p7.cpp Tue Feb 7 21:30:13 2017
@@ -1,11 +1,9 @@
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify %s -Wshadow-field
-// expected-no-diagnostics
-
-struct A { int n; };
-struct B { float n; };
+struct A { int n; }; // expected-note {{declared here}}
+struct B { float n; }; // expected-note {{declared here}}
struct C : A, B {};
struct D : virtual C {};
-struct E : virtual C { char n; };
+struct E : virtual C { char n; }; // expected-warning {{non-static data member 'n' of 'E' shadows member inherited from type 'A'}} expected-warning {{non-static data member 'n' of 'E' shadows member inherited from type 'B'}}
struct F : D, E {} f;
char &k = f.n;
More information about the cfe-commits
mailing list