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