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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits