r294401 - Sema: add warning for c++ member variable shadowing
Sean Silva via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 7 19:46:31 PST 2017
Nice!
On Tue, Feb 7, 2017 at 7:30 PM, Saleem Abdulrasool via cfe-commits <
cfe-commits at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170207/40f06ab9/attachment-0001.html>
More information about the cfe-commits
mailing list