commit 145a15861519713c1cd1db5bb961782547943261
Author: James Sun <jamessun@fb.com>
Date:   Tue Jan 24 14:08:45 2017 -0800

    Add warning for c++ member variable shadowing
    
    For cases where we really don't want to shadow member variables, e.g.
    
    struct a {
      int foo; // A public or protected field
    }
    
    class b : a {
      int foo; // Generate a warning
    }
    
    This patch
    (1) adds a member variable shadowing checking, and
    (2) incorporates it to the unit tests.

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 6290172..9f698fb 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8926,4 +8926,8 @@ 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<ShadowIvar>;
+
 } // end of sema component.
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 1d3b273..4849a6f 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -10061,6 +10061,11 @@ private:
   void CheckBitFieldInitialization(SourceLocation InitLoc, FieldDecl *Field,
                                    Expr *Init);
 
+  /// Check if there is a field shadowing.
+  void CheckShadowInheritedFields(const SourceLocation &Loc,
+                                  StringRef 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);
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index a70e16c..f147cce 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -2758,6 +2758,38 @@ static AttributeList *getMSPropertyAttr(AttributeList *list) {
   return nullptr;
 }
 
+// Check if there is a field shadowing.
+void Sema::CheckShadowInheritedFields(const SourceLocation &Loc,
+                                      StringRef FieldName,
+                                      const CXXRecordDecl *RD) {
+  if (Diags.isIgnored(diag::warn_shadow_field, Loc))
+    return;
+
+  std::set<StringRef> bases;
+  auto FieldShadowed = [&](const CXXBaseSpecifier *Specifier,
+                           CXXBasePath &Path) {
+    const auto baseName = Specifier->getType()->getAsCXXRecordDecl()->getName();
+    if (bases.find(baseName) != bases.end()) {
+      // Avoid printing duplicated base classes.
+      return false;
+    }
+    for (const auto *Field : Specifier->getType()->getAsCXXRecordDecl()->fields()) {
+      if ((Field->getAccess() == AS_public || Field->getAccess() == AS_protected) &&
+          Field->getName() == FieldName) {
+        assert(bases.find(baseName) == bases.end());
+        bases.emplace(baseName);
+        Diag(Loc, diag::warn_shadow_field)
+          << FieldName << RD->getName() << baseName;
+        return true;
+      }
+    }
+    return false;
+  };
+  CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/false,
+                     /*DetectVirtual=*/true);
+  RD->lookupInBases(FieldShadowed, Paths);
+}
+
 /// 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 +2989,11 @@ Sema::ActOnCXXMemberDeclarator(Scope *S, AccessSpecifier AS, Declarator &D,
       if (!Member)
         return nullptr;
     }
+
+    // Check for any possible shadowed member variables
+    if (const auto *RD = cast<CXXRecordDecl>(CurContext))
+      CheckShadowInheritedFields(Loc, Name.getAsString(), RD);
+
   } else {
     Member = HandleDeclarator(S, D, TemplateParameterLists);
     if (!Member)
diff --git a/test/Analysis/padding_cpp.cpp b/test/Analysis/padding_cpp.cpp
index df2f2a8..6b9811f 100644
--- a/test/Analysis/padding_cpp.cpp
+++ b/test/Analysis/padding_cpp.cpp
@@ -10,11 +10,11 @@ struct BigCharArray2 { // no-warning
 // xxxexpected-warning@+1{{Excessive padding in 'struct LowAlignmentBase'}}
 struct LowAlignmentBase : public BigCharArray2 {
   int i;
-  char c;
+  char c; // expected-warning{{non-static data member 'c' of 'LowAlignmentBase' shadows member inherited from type 'BigCharArray2'}}
 };
 
 struct CorrectLowAlignmentBase : public BigCharArray2 { // no-warning
-  char c;
+  char c; // expected-warning{{non-static data member 'c' of 'CorrectLowAlignmentBase' shadows member inherited from type 'BigCharArray2'}}
   int i;
 };
 
diff --git a/test/CXX/class.derived/class.member.lookup/p10.cpp b/test/CXX/class.derived/class.member.lookup/p10.cpp
new file mode 100644
index 0000000..aa79627
--- /dev/null
+++ b/test/CXX/class.derived/class.member.lookup/p10.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+class A {
+public:
+  int x;
+protected:
+  int y;
+private:
+  int z;
+};
+
+class B : A {
+};
+
+class C : A {
+};
+
+struct W {
+  int w;
+};
+
+class U : W {
+};
+
+class V : W {
+};
+
+class D {
+public:
+  char w;
+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'}}
+};
+
diff --git a/test/CXX/class.derived/class.member.lookup/p6.cpp b/test/CXX/class.derived/class.member.lookup/p6.cpp
index 7239881..7bc3abb 100644
--- a/test/CXX/class.derived/class.member.lookup/p6.cpp
+++ b/test/CXX/class.derived/class.member.lookup/p6.cpp
@@ -16,9 +16,9 @@ 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 { };
diff --git a/test/CXX/class.derived/class.member.lookup/p7.cpp b/test/CXX/class.derived/class.member.lookup/p7.cpp
index a785e0f..6198ee2 100644
--- a/test/CXX/class.derived/class.member.lookup/p7.cpp
+++ b/test/CXX/class.derived/class.member.lookup/p7.cpp
@@ -1,11 +1,10 @@
 // RUN: %clang_cc1 -verify %s
 
-// expected-no-diagnostics
 
 struct A { int n; };
 struct B { float n; };
 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;
diff --git a/test/CXX/dcl.decl/dcl.decomp/p4.cpp b/test/CXX/dcl.decl/dcl.decomp/p4.cpp
index c461eb6..8f0593f 100644
--- a/test/CXX/dcl.decl/dcl.decomp/p4.cpp
+++ b/test/CXX/dcl.decl/dcl.decomp/p4.cpp
@@ -53,7 +53,7 @@ namespace AnonymousMember {
 
 namespace MultipleClasses {
   struct B : A {
-    int a;
+    int a;  // expected-warning {{non-static data member 'a' of 'B' shadows member inherited from type 'A'}}
   };
 
   struct C { int a; };
diff --git a/test/CXX/drs/dr5xx.cpp b/test/CXX/drs/dr5xx.cpp
index 89e404f..de81cb0 100644
--- a/test/CXX/drs/dr5xx.cpp
+++ b/test/CXX/drs/dr5xx.cpp
@@ -655,7 +655,7 @@ namespace dr568 { // dr568: yes c++11
   // too.
   struct x { int y; };
   class trivial : x {
-    x y;
+    x y;  // expected-warning {{non-static data member 'y' of 'trivial' shadows member inherited from type 'x'}}
   public:
     int n;
   };
diff --git a/test/SemaCXX/class-layout.cpp b/test/SemaCXX/class-layout.cpp
index 96552de..10ce356 100644
--- a/test/SemaCXX/class-layout.cpp
+++ b/test/SemaCXX/class-layout.cpp
@@ -1,6 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++98 -Wno-inaccessible-base
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base
-// expected-no-diagnostics
 
 #define SA(n, p) int a##n[(p) ? 1 : -1]
 
@@ -69,7 +68,7 @@ SA(8, sizeof(B) == 2);
 struct C { bool iv0 : 1; };
 SA(9, sizeof(C) == 1);  
 
-struct D : C { bool iv0 : 1; };
+struct D : C { bool iv0 : 1; }; // expected-warning {{non-static data member 'iv0' of 'D' shadows member inherited from type 'C'}}
 SA(10, sizeof(D) == 2);
 
 }
@@ -519,7 +518,7 @@ namespace test17 {
   };
     
   struct might_use_tail_padding : public A, public B, public C, public D {
-    char a;
+    char a; // expected-warning {{non-static data member 'a' of 'might_use_tail_padding' shadows member inherited from type 'A'}}
   };
   SA(0, sizeof(might_use_tail_padding) == 512);
 }
diff --git a/test/SemaCXX/constant-expression-cxx11.cpp b/test/SemaCXX/constant-expression-cxx11.cpp
index 0668324..14bd2d4 100644
--- a/test/SemaCXX/constant-expression-cxx11.cpp
+++ b/test/SemaCXX/constant-expression-cxx11.cpp
@@ -27,8 +27,8 @@ struct MemberZero {
 namespace DerivedToVBaseCast {
 
   struct U { int n; };
-  struct V : U { int n; };
-  struct A : virtual V { int n; };
+  struct V : U { int n; };  // expected-warning {{non-static data member 'n' of 'V' shadows member inherited from type 'U'}}
+  struct A : virtual V { int n; }; // expected-warning {{non-static data member 'n' of 'A' shadows member inherited from type 'V'}}
   struct Aa { int n; };
   struct B : virtual A, Aa {};
   struct C : virtual A, Aa {};