Do not check for parameters shadowing fields in function declarations

[aaron.ballman]

Summary

This patch changes the way we handle -Wshadow-field so that we do not issue diagnostics for parameters in function declarations (as those are harmless), but we continue to diagnose parameters of function definitions.

[lebedev.ri]

test/SemaCXX/warn-shadow.cpp

236

Can you please also add one function with out-of-line definition?

[aaron.ballman]

Added a test with an out-of-line definition.

[aaron.ballman]

test/SemaCXX/warn-shadow.cpp

236

Gladly!

[steveire]

test/SemaCXX/warn-shadow.cpp

236

Is this redundant now that you've added G?

[aaron.ballman]

test/SemaCXX/warn-shadow.cpp

236

Sort of. G eventually has a body whereas F does not -- one of the gates on this logic is a function named willHaveBody(), so a bit of redundancy here seems reasonable.

[george.karpenkov]

Thanks!

[aaron.ballman]

Committed in r348400.

lib/Sema/SemaChecking.cpp

     // Parameters with the pass_object_size attribute only need to be marked
     // constant at function definitions. Because we lack information about
     // whether we're on a declaration or definition when we're instantiating the
     // attribute, we need to check for constness here.
     if (const auto *Attr = Param->getAttr<PassObjectSizeAttr>())
       if (!Param->getType().isConstQualified())
         Diag(Param->getLocation(), diag::err_attribute_pointers_only)
             << Attr->getSpelling() << 1;
+
+    // Check for parameter names shadowing fields from the class.
+    if (LangOpts.CPlusPlus && !Param->isInvalidDecl()) {
+      // The owning context for the parameter should be the function, but we
+      // want to see if this function's declaration context is a record.
+      DeclContext *DC = Param->getDeclContext();
+      if (DC && DC->isFunctionOrMethod()) {
+        if (auto *RD = dyn_cast<CXXRecordDecl>(DC->getParent()))
+          CheckShadowInheritedFields(Param->getLocation(), Param->getDeclName(),
+                                     RD, /*DeclIsField*/ false);
+      }
+    }
   }
 
   return HasInvalidParm;
 }
 
 /// A helper function to get the alignment of a Decl referred to by DeclRefExpr
 /// or MemberExpr.
 static CharUnits getDeclAlign(Expr *E, CharUnits TypeAlign,

lib/Sema/SemaDecl.cpp

         Diag(PrevDecl->getLocation(), diag::note_previous_declaration);
 
         // Recover by removing the name
         II = nullptr;
         D.SetIdentifier(nullptr, D.getIdentifierLoc());
         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
   // the enclosing context.  This prevents them from accidentally
   // looking like class members in C++.
   ParmVarDecl *New =
       CheckParameter(Context.getTranslationUnitDecl(), D.getBeginLoc(),
                      D.getIdentifierLoc(), II, parmDeclType, TInfo, SC);

test/SemaCXX/warn-shadow.cpp

     void g(int a) {}
     A() { int a; }
   };
 }
 }
 
 namespace PR34120 {
 struct A {
-  int B; // expected-note {{declared here}}
+  int B; // expected-note 2 {{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
   }
+  void F(int B); // Ok, declaration; not definition.
+  void G(int B);
 };
 
+void C::G(int B) { // expected-warning {{parameter 'B' shadows member inherited from type 'A'}}
+}
+
 class Private {
   int B;
 };
 class Derived : Private {
   void D(int B) {} // Ok
 };
 
 struct Static {