-Wshadow should only warn about parameter declarations when we're

entering a function or block definition, not on every single declaration.
Unfortunately we don't have previous-lookup results around when it's time
to make this decision, so we have to redo the lookup.  The alternative is
to use delayed diagnostics.

llvm-svn: 99172

clang/lib/Sema/Sema.h

@@ -783,7 +783,8 @@ public:
                                         const LookupResult &Previous,
                                         Scope *S);
   void DiagnoseFunctionSpecifiers(Declarator& D);
-  void DiagnoseShadow(Scope *S, Declarator &D, const LookupResult& R);
+  void CheckShadow(Scope *S, VarDecl *D, const LookupResult& R);
+  void CheckShadow(Scope *S, VarDecl *D);
   NamedDecl* ActOnTypedefDeclarator(Scope* S, Declarator& D, DeclContext* DC,
                                     QualType R, TypeSourceInfo *TInfo,
                                     LookupResult &Previous, bool &Redeclaration);

clang/lib/Sema/SemaDecl.cpp

@@ -2405,7 +2405,7 @@ Sema::ActOnVariableDeclarator(Scope* S, Declarator& D, DeclContext* DC,
 
   // Diagnose shadowed variables before filtering for scope.
   if (!D.getCXXScopeSpec().isSet())
-    DiagnoseShadow(S, D, Previous);
+    CheckShadow(S, NewVD, Previous);
 
   // Don't consider existing declarations that are in a different
   // scope and are out-of-semantic-context declarations (if the new
@@ -2458,19 +2458,16 @@ Sema::ActOnVariableDeclarator(Scope* S, Declarator& D, DeclContext* DC,
   return NewVD;
 }
 
-/// \brief Diagnose variable or built-in function shadowing.
+/// \brief Diagnose variable or built-in function shadowing.  Implements
+/// -Wshadow.
 ///
-/// This method is called as soon as a NamedDecl materializes to check
+/// This method is called whenever a VarDecl is added to a "useful"
-/// if it shadows another local or global variable, or a built-in function.
+/// scope.
-///
-/// For performance reasons, the lookup results are reused from the calling
-/// context.
 ///
 /// \param S the scope in which the shadowing name is being declared
 /// \param R the lookup of the name
 ///
-void Sema::DiagnoseShadow(Scope *S, Declarator &D,
+void Sema::CheckShadow(Scope *S, VarDecl *D, const LookupResult& R) {
-                          const LookupResult& R) {
   // Return if warning is ignored.
   if (Diags.getDiagnosticLevel(diag::warn_decl_shadow) == Diagnostic::Ignored)
     return;
@@ -2524,6 +2521,14 @@ void Sema::DiagnoseShadow(Scope *S, Declarator &D,
   Diag(ShadowedDecl->getLocation(), diag::note_previous_declaration);
 }
 
+/// \brief Check -Wshadow without the advantage of a previous lookup.
+void Sema::CheckShadow(Scope *S, VarDecl *D) {
+  LookupResult R(*this, D->getDeclName(), D->getLocation(),
+                 Sema::LookupOrdinaryName, Sema::ForRedeclaration);
+  LookupName(R, S);
+  CheckShadow(S, D, R);
+}
+
 /// \brief Perform semantic checking on a newly-created variable
 /// declaration.
 ///
@@ -3984,8 +3989,6 @@ Sema::ActOnParamDeclarator(Scope *S, Declarator &D) {
         II = 0;
         D.SetIdentifier(0, D.getIdentifierLoc());
         D.setInvalidType(true);
-      } else {
-        DiagnoseShadow(S, D, R);
       }
     }
   }
@@ -4213,14 +4216,21 @@ Sema::DeclPtrTy Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, DeclPtrTy D) {
   // Check the validity of our function parameters
   CheckParmsForFunctionDef(FD);
 
+  bool ShouldCheckShadow =
+    Diags.getDiagnosticLevel(diag::warn_decl_shadow) != Diagnostic::Ignored;
+
   // Introduce our parameters into the function scope
   for (unsigned p = 0, NumParams = FD->getNumParams(); p < NumParams; ++p) {
     ParmVarDecl *Param = FD->getParamDecl(p);
     Param->setOwningFunction(FD);
 
     // If this has an identifier, add it to the scope stack.
-    if (Param->getIdentifier() && FnBodyScope)
+    if (Param->getIdentifier() && FnBodyScope) {
+      if (ShouldCheckShadow)
+        CheckShadow(FnBodyScope, Param);
+
       PushOnScopeChains(Param, FnBodyScope);
+    }
   }
 
   // Checking attributes of current function definition

clang/lib/Sema/SemaExpr.cpp

@@ -6905,13 +6905,21 @@ void Sema::ActOnBlockArguments(Declarator &ParamInfo, Scope *CurScope) {
                                CurBlock->Params.size());
   CurBlock->TheDecl->setIsVariadic(CurBlock->isVariadic);
   ProcessDeclAttributes(CurScope, CurBlock->TheDecl, ParamInfo);
+
+  bool ShouldCheckShadow =
+    Diags.getDiagnosticLevel(diag::warn_decl_shadow) != Diagnostic::Ignored;
+
   for (BlockDecl::param_iterator AI = CurBlock->TheDecl->param_begin(),
          E = CurBlock->TheDecl->param_end(); AI != E; ++AI) {
     (*AI)->setOwningFunction(CurBlock->TheDecl);
 
     // If this has an identifier, add it to the scope stack.
-    if ((*AI)->getIdentifier())
+    if ((*AI)->getIdentifier()) {
+      if (ShouldCheckShadow)
+        CheckShadow(CurBlock->TheScope, *AI);
+
       PushOnScopeChains(*AI, CurBlock->TheScope);
+    }
   }
 
   // Check for a valid sentinel attribute on this block.

clang/test/Sema/warn-shadow.c

@@ -43,3 +43,8 @@ void test3(void) {
 
 void test4(int i) { // expected-warning {{declaration shadows a variable in the global scope}}
 }
+
+// Don't warn about shadowing for function declarations.
+void test5(int i);
+void test6(void (*f)(int i)) {}
+void test7(void *context, void (*callback)(void *context)) {}