Nice! On Tue, Feb 7, 2017 at 7:30 PM, Saleem Abdulrasool via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> 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", > [ShadowFieldInConstructorModifi > ed]>; > @@ -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 >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
