Diagnose parameter names that shadow the names of inherited fields under -Wshadow...

This addresses PR34120. Note, unlike GCC, we take into account the accessibility of the field when deciding whether to warn or not.

llvm-svn: 346041

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8403278..d322457 100644 (file)
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9467,16 +9467,15 @@ def warn_block_literal_qualifiers_on_omitted_return_type : Warning<
   InGroup<IgnoredQualifiers>;
 
 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">;
-
-def err_multiversion_required_in_redecl : Error<
+  "__final is a GNU extension, consider using C++11 final">,\r
+  InGroup<GccCompat>;\r
+\r
+def warn_shadow_field : Warning<\r
+  "%select{parameter|non-static data member}3 %0 %select{|of %1 }3shadows "\r
+  "member inherited from type %2">, InGroup<ShadowField>, DefaultIgnore;\r
+def note_shadow_field : Note<"declared here">;\r
+\r
+def err_multiversion_required_in_redecl : Error<\r
   "function declaration is missing %select{'target'|'cpu_specific' or "
   "'cpu_dispatch'}0 attribute in a multiversioned function">;
 def note_multiversioning_caused_here : Note<

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 4740cca..e465524 100644 (file)
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10588,7 +10588,8 @@ private:
   /// Check if there is a field shadowing.
   void CheckShadowInheritedFields(const SourceLocation &Loc,
                                   DeclarationName FieldName,
-                                  const CXXRecordDecl *RD);
+                                  const CXXRecordDecl *RD,
+                                  bool DeclIsField = true);
 
   /// Check if the given expression contains 'break' or 'continue'
   /// statement that produces control flow different from GCC.

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index bf15e25..f8068dd 100644 (file)
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -12366,6 +12366,13 @@ Decl *Sema::ActOnParamDeclarator(Scope *S, Declarator &D) {
         D.setInvalidType(true);
       }
     }
+
+    if (LangOpts.CPlusPlus) {
+      DeclarationNameInfo DNI = GetNameForDeclarator(D);
+      if (auto *RD = dyn_cast<CXXRecordDecl>(CurContext))
+        CheckShadowInheritedFields(DNI.getLoc(), DNI.getName(), RD,
+                                   /*DeclIsField*/ false);
+    }
   }
 
   // Temporarily put parameter variables in the translation unit, not

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 51a4090..af4be99 100644 (file)
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -2832,13 +2832,14 @@ static const ParsedAttr *getMSPropertyAttr(const ParsedAttributesView &list) {
   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;
-
+// Check if there is a field shadowing.\r
+void Sema::CheckShadowInheritedFields(const SourceLocation &Loc,\r
+                                      DeclarationName FieldName,\r
+                                      const CXXRecordDecl *RD,\r
+                                      bool DeclIsField) {\r
+  if (Diags.isIgnored(diag::warn_shadow_field, Loc))\r
+    return;\r
+\r
   // To record a shadowed field in a base
   std::map<CXXRecordDecl*, NamedDecl*> Bases;
   auto FieldShadowed = [&](const CXXBaseSpecifier *Specifier,
@@ -2872,13 +2873,13 @@ void Sema::CheckShadowInheritedFields(const SourceLocation &Loc,
       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 << RD << Base;
-      Diag(BaseField->getLocation(), diag::note_shadow_field);
-      Bases.erase(It);
-    }
+    if (AS_none !=\r
+        CXXRecordDecl::MergeAccess(P.Access, BaseField->getAccess())) {\r
+      Diag(Loc, diag::warn_shadow_field)\r
+        << FieldName << RD << Base << DeclIsField;\r
+      Diag(BaseField->getLocation(), diag::note_shadow_field);\r
+      Bases.erase(It);\r
+    }\r
   }
 }
 

diff --git a/clang/test/SemaCXX/warn-shadow.cpp b/clang/test/SemaCXX/warn-shadow.cpp
index 33203f0..283fb6d 100644 (file)
--- a/clang/test/SemaCXX/warn-shadow.cpp
+++ b/clang/test/SemaCXX/warn-shadow.cpp
@@ -59,13 +59,13 @@ class A {
   // expected-warning-re@+1 4 {{constructor parameter 'f{{[0-4]}}' shadows the field 'f{{[0-9]}}' of 'A'}}
   A(int f1, int f2, int f3, int f4, double overload_dummy) {}
 
-  void test() {
-    char *field; // expected-warning {{declaration shadows a field of 'A'}}
-    char *data; // expected-warning {{declaration shadows a static data member of 'A'}}
+  void test() {\r
+    char *field; // expected-warning {{declaration shadows a field of 'A'}}\r
+    char *data; // expected-warning {{declaration shadows a static data member of 'A'}}\r
     char *a1; // no warning 
-    char *a2; // no warning
-    char *jj; // no warning
-    char *jjj; // no warning
+    char *a2; // no warning\r
+    char *jj; // no warning\r
+    char *jjj; // no warning\r
   }
 
   void test2() {
@@ -196,14 +196,14 @@ void avoidWarningWhenRedefining(int b) { // expected-note {{previous definition
   int k; // expected-note {{previous definition is here}}
   typedef int k; // expected-error {{redefinition of 'k'}}
 
-  using l=char; // no warning or error.
-  using l=char; // no warning or error.
-  typedef char l; // no warning or error.
+  using l=char; // no warning or error.\r
+  using l=char; // no warning or error.\r
+  typedef char l; // no warning or error.\r
  
   typedef char n; // no warning or error. 
-  typedef char n; // no warning or error.
-  using n=char; // no warning or error.
-}
+  typedef char n; // no warning or error.\r
+  using n=char; // no warning or error.\r
+}\r
 
 }
 
@@ -219,9 +219,37 @@ void f(int a) {
   struct A {
     void g(int a) {}
     A() { int a; }
-  };
-}
-}
+  };\r
+}\r
+}\r
+\r
+namespace PR34120 {\r
+struct A {\r
+  int B; // expected-note {{declared here}}\r
+};\r
+\r
+class C : public A {\r
+  void D(int B) {} // expected-warning {{parameter 'B' shadows member inherited from type 'A'}}\r
+  void E() {\r
+    extern void f(int B); // Ok\r
+  }\r
+};\r
+\r
+class Private {\r
+  int B;\r
+};\r
+class Derived : Private {\r
+  void D(int B) {} // Ok\r
+};\r
+\r
+struct Static {\r
+  static int B;\r
+};\r
+\r
+struct Derived2 : Static {\r
+  void D(int B) {}\r
+};\r
+}\r
 
 int PR24718;
 enum class X { PR24718 }; // Ok, not shadowing