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

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 2 14:04:44 PDT 2018


Author: aaronballman
Date: Fri Nov  2 14:04:44 2018
New Revision: 346041

URL: http://llvm.org/viewvc/llvm-project?rev=346041&view=rev
Log:
Diagnose parameter names that shadow the names of inherited fields under -Wshadow-field.

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

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
    cfe/trunk/test/SemaCXX/warn-shadow.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=346041&r1=346040&r2=346041&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Nov  2 14:04:44 2018
@@ -9467,16 +9467,15 @@ def warn_block_literal_qualifiers_on_omi
   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">,
+  InGroup<GccCompat>;
+
+def warn_shadow_field : Warning<
+  "%select{parameter|non-static data member}3 %0 %select{|of %1 }3shadows "
+  "member inherited from type %2">, InGroup<ShadowField>, DefaultIgnore;
+def note_shadow_field : Note<"declared here">;
+
+def err_multiversion_required_in_redecl : Error<
   "function declaration is missing %select{'target'|'cpu_specific' or "
   "'cpu_dispatch'}0 attribute in a multiversioned function">;
 def note_multiversioning_caused_here : Note<

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=346041&r1=346040&r2=346041&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Nov  2 14:04:44 2018
@@ -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.

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=346041&r1=346040&r2=346041&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Nov  2 14:04:44 2018
@@ -12366,6 +12366,13 @@ Decl *Sema::ActOnParamDeclarator(Scope *
         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

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=346041&r1=346040&r2=346041&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Fri Nov  2 14:04:44 2018
@@ -2832,13 +2832,14 @@ static const ParsedAttr *getMSPropertyAt
   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.
+void Sema::CheckShadowInheritedFields(const SourceLocation &Loc,
+                                      DeclarationName FieldName,
+                                      const CXXRecordDecl *RD,
+                                      bool DeclIsField) {
+  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,
@@ -2872,13 +2873,13 @@ void Sema::CheckShadowInheritedFields(co
       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 !=
+        CXXRecordDecl::MergeAccess(P.Access, BaseField->getAccess())) {
+      Diag(Loc, diag::warn_shadow_field)
+        << FieldName << RD << Base << DeclIsField;
+      Diag(BaseField->getLocation(), diag::note_shadow_field);
+      Bases.erase(It);
+    }
   }
 }
 

Modified: cfe/trunk/test/SemaCXX/warn-shadow.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-shadow.cpp?rev=346041&r1=346040&r2=346041&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-shadow.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-shadow.cpp Fri Nov  2 14:04:44 2018
@@ -59,13 +59,13 @@ class A {
   // expected-warning-re at +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() {
+    char *field; // expected-warning {{declaration shadows a field of 'A'}}
+    char *data; // expected-warning {{declaration shadows a static data member of 'A'}}
     char *a1; // no warning 
-    char *a2; // no warning
-    char *jj; // no warning
-    char *jjj; // no warning
+    char *a2; // no warning
+    char *jj; // no warning
+    char *jjj; // no warning
   }
 
   void test2() {
@@ -196,14 +196,14 @@ void avoidWarningWhenRedefining(int b) {
   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.
+  using l=char; // no warning or error.
+  typedef char l; // no warning or error.
  
   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.
+  using n=char; // no warning or error.
+}
 
 }
 
@@ -219,9 +219,37 @@ void f(int a) {
   struct A {
     void g(int a) {}
     A() { int a; }
-  };
-}
-}
+  };
+}
+}
+
+namespace PR34120 {
+struct A {
+  int B; // expected-note {{declared here}}
+};
+
+class C : public A {
+  void D(int B) {} // expected-warning {{parameter 'B' shadows member inherited from type 'A'}}
+  void E() {
+    extern void f(int B); // Ok
+  }
+};
+
+class Private {
+  int B;
+};
+class Derived : Private {
+  void D(int B) {} // Ok
+};
+
+struct Static {
+  static int B;
+};
+
+struct Derived2 : Static {
+  void D(int B) {}
+};
+}
 
 int PR24718;
 enum class X { PR24718 }; // Ok, not shadowing




More information about the cfe-commits mailing list