Avoid -Wshadow warnings for shadowed variables that aren't captured by lambdas with a default capture specifier

[arphaman]

Summary

This is a follow-up patch to r286354. This patch avoids the -Wshadow warning for variables which shadow variables that aren't captured by lambdas with a default capture specifier. It provides an additional note that points to location of the capture.

The old behaviour is preserved with -Wshadow-all.

This fixes PR 21426 fully.

[rnk]

lgtm, thanks

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def err_access_lambda_capture : Error<
     // The ERRORs represent other special members that aren't constructors, in
     // hopes that someone will bother noticing and reporting if they appear
     "capture of variable '%0' as type %1 calls %select{private|protected}3 "
     "%select{default |copy |move |*ERROR* |*ERROR* |*ERROR* |}2constructor">,
     AccessControl;
   def note_lambda_to_block_conv : Note<
     "implicit capture of lambda object due to conversion to block pointer "
     "here">;
-  def note_var_explicitly_captured_here : Note<"variable %0 is explicitly "
-    "captured here">;
+  def note_var_explicitly_captured_here : Note<"variable %0 is"
+    "%select{| explicitly}1 captured here">;
 
   // C++14 lambda init-captures.
   def warn_cxx11_compat_init_capture : Warning<
     "initialized lambda captures are incompatible with C++ standards "
     "before C++14">, InGroup<CXXPre14Compat>, DefaultIgnore;
   def ext_init_capture : ExtWarn<
     "initialized lambda captures are a C++14 extension">, InGroup<CXX14>;
   def err_init_capture_no_expression : Error<

cfe/trunk/include/clang/Sema/ScopeInfo.h

public:
   ///  enclosing lambda if all intervening lambdas can capture the variable.
 
   llvm::SmallVector<Expr*, 4> PotentiallyCapturingExprs;
 
   /// \brief Contains all variable-referring-expressions that refer
   ///  to local variables that are usable as constant expressions and
   ///  do not involve an odr-use (they may still need to be captured
   ///  if the enclosing full-expression is instantiation dependent).
-  llvm::SmallSet<Expr*, 8> NonODRUsedCapturingExprs; 
+  llvm::SmallSet<Expr *, 8> NonODRUsedCapturingExprs;
 
+  /// Contains all of the variables defined in this lambda that shadow variables
+  /// that were defined in parent contexts. Used to avoid warnings when the
+  /// shadowed variables are uncaptured by this lambda.
+  struct ShadowedOuterDecl {
+    const VarDecl *VD;
+    const VarDecl *ShadowedDecl;
+  };
+  llvm::SmallVector<ShadowedOuterDecl, 4> ShadowingDecls;
+
   SourceLocation PotentialThisCaptureLocation;
 
   LambdaScopeInfo(DiagnosticsEngine &Diag)
     : CapturingScopeInfo(Diag, ImpCap_None), Lambda(nullptr),
       CallOperator(nullptr), NumExplicitCaptures(0), Mutable(false),
       ExplicitParams(false), Cleanup{},
       ContainsUnexpandedParameterPack(false), AutoTemplateParameterDepth(0),
       GLTemplateParameterList(nullptr) {

cfe/trunk/include/clang/Sema/Sema.h

public:
   void DiagnoseFunctionSpecifiers(const DeclSpec &DS);
   void CheckShadow(Scope *S, VarDecl *D, const LookupResult& R);
   void CheckShadow(Scope *S, VarDecl *D);
 
   /// Warn if 'E', which is an expression that is about to be modified, refers
   /// to a shadowing declaration.
   void CheckShadowingDeclModification(Expr *E, SourceLocation Loc);
 
+  void DiagnoseShadowingLambdaDecls(const sema::LambdaScopeInfo *LSI);
+
 private:
   /// Map of current shadowing declarations to shadowed declarations. Warn if
   /// it looks like the user is trying to modify the shadowing declaration.
   llvm::DenseMap<const NamedDecl *, const NamedDecl *> ShadowingDecls;
 
 public:
   void CheckCastAlign(Expr *Op, QualType T, SourceRange TRange);
   void handleTagNumbering(const TagDecl *Tag, Scope *TagScope);

cfe/trunk/lib/Sema/SemaDecl.cpp

if (VarDecl *shadowedVar = dyn_cast<VarDecl>(ShadowedDecl))
     }
 
   DeclContext *OldDC = ShadowedDecl->getDeclContext();
 
   unsigned WarningDiag = diag::warn_decl_shadow;
   SourceLocation CaptureLoc;
   if (isa<VarDecl>(ShadowedDecl) && NewDC && isa<CXXMethodDecl>(NewDC)) {
     if (const auto *RD = dyn_cast<CXXRecordDecl>(NewDC->getParent())) {
+      if (RD->isLambda() && OldDC->Encloses(NewDC->getLexicalParent())) {
+        if (RD->getLambdaCaptureDefault() == LCD_None) {
           // Try to avoid warnings for lambdas with an explicit capture list.
-      if (RD->isLambda() && OldDC->Encloses(NewDC->getLexicalParent()) &&
-          RD->getLambdaCaptureDefault() == LCD_None) {
           const auto *LSI = cast<LambdaScopeInfo>(getCurFunction());
           // Warn only when the lambda captures the shadowed decl explicitly.
           CaptureLoc = getCaptureLocation(LSI, cast<VarDecl>(ShadowedDecl));
           if (CaptureLoc.isInvalid())
             WarningDiag = diag::warn_decl_shadow_uncaptured_local;
+        } else {
+          // Remember that this was shadowed so we can avoid the warning if the
+          // shadowed decl isn't captured and the warning settings allow it.
+          cast<LambdaScopeInfo>(getCurFunction())
+              ->ShadowingDecls.push_back({D, cast<VarDecl>(ShadowedDecl)});
+          return;
+        }
       }
     }
   }
 
   // Only warn about certain kinds of shadowing for class members.
   if (NewDC && NewDC->isRecord()) {
     // In particular, don't warn about shadowing non-class members.
     if (!OldDC->isRecord())
void Sema::CheckShadow(Scope *S, VarDecl *D, const LookupResult& R) {
   DeclarationName Name = R.getLookupName();
 
   // Emit warning and note.
   if (getSourceManager().isInSystemMacro(R.getNameLoc()))
     return;
   ShadowedDeclKind Kind = computeShadowedDeclKind(ShadowedDecl, OldDC);
   Diag(R.getNameLoc(), WarningDiag) << Name << Kind << OldDC;
   if (!CaptureLoc.isInvalid())
-    Diag(CaptureLoc, diag::note_var_explicitly_captured_here) << Name;
+    Diag(CaptureLoc, diag::note_var_explicitly_captured_here)
+        << Name << /*explicitly*/ 1;
   Diag(ShadowedDecl->getLocation(), diag::note_previous_declaration);
 }
 
+/// Diagnose shadowing for variables shadowed in the lambda record \p LambdaRD
+/// when these variables are captured by the lambda.
+void Sema::DiagnoseShadowingLambdaDecls(const LambdaScopeInfo *LSI) {
+  for (const auto &Shadow : LSI->ShadowingDecls) {
+    const VarDecl *ShadowedDecl = Shadow.ShadowedDecl;
+    // Try to avoid the warning when the shadowed decl isn't captured.
+    SourceLocation CaptureLoc = getCaptureLocation(LSI, ShadowedDecl);
+    const DeclContext *OldDC = ShadowedDecl->getDeclContext();
+    Diag(Shadow.VD->getLocation(), CaptureLoc.isInvalid()
+                                       ? diag::warn_decl_shadow_uncaptured_local
+                                       : diag::warn_decl_shadow)
+        << Shadow.VD->getDeclName()
+        << computeShadowedDeclKind(ShadowedDecl, OldDC) << OldDC;
+    if (!CaptureLoc.isInvalid())
+      Diag(CaptureLoc, diag::note_var_explicitly_captured_here)
+          << Shadow.VD->getDeclName() << /*explicitly*/ 0;
+    Diag(ShadowedDecl->getLocation(), diag::note_previous_declaration);
+  }
+}
+
 /// \brief Check -Wshadow without the advantage of a previous lookup.
 void Sema::CheckShadow(Scope *S, VarDecl *D) {
   if (Diags.isIgnored(diag::warn_decl_shadow, D->getLocation()))
     return;
 
   LookupResult R(*this, D->getDeclName(), D->getLocation(),
                  Sema::LookupOrdinaryName, Sema::ForRedeclaration);
   LookupName(R, S);

cfe/trunk/lib/Sema/SemaLambda.cpp

if (getLangOpts().CPlusPlus1z && !CallOperator->isInvalidDecl() &&
       !CallOperator->isConstexpr() &&
       !Class->getDeclContext()->isDependentContext()) {
     TentativeAnalysisScope DiagnosticScopeGuard(*this);
     CallOperator->setConstexpr(
         CheckConstexprFunctionDecl(CallOperator) &&
         CheckConstexprFunctionBody(CallOperator, CallOperator->getBody()));
   }
 
+  // Emit delayed shadowing warnings now that the full capture list is known.
+  DiagnoseShadowingLambdaDecls(LSI);
+
   if (!CurContext->isDependentContext()) {
     switch (ExprEvalContexts.back().Context) {
     // C++11 [expr.prim.lambda]p2:
     //   A lambda-expression shall not appear in an unevaluated operand
     //   (Clause 5).
     case Unevaluated:
     case UnevaluatedAbstract:
     // C++1y [expr.const]p2:

cfe/trunk/test/SemaCXX/warn-shadow-in-lambdas.cpp

 // RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -Wshadow -D AVOID %s
 // RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -Wshadow -Wshadow-uncaptured-local %s
 // RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -Wshadow-all %s
 
 void foo(int param) { // expected-note 1+ {{previous declaration is here}}
   int var = 0; // expected-note 1+ {{previous declaration is here}}
 
-  // Warn for lambdas with a default capture specifier.
+  // Avoid warnings for variables that aren't implicitly captured.
   {
+#ifdef AVOID
+    auto f1 = [=] { int var = 1; };  // no warning
+    auto f2 = [&] { int var = 2; };  // no warning
+    auto f3 = [=] (int param) { ; }; // no warning
+    auto f4 = [&] (int param) { ; }; // no warning
+#else
     auto f1 = [=] { int var = 1; };  // expected-warning {{declaration shadows a local variable}}
     auto f2 = [&] { int var = 2; };  // expected-warning {{declaration shadows a local variable}}
     auto f3 = [=] (int param) { ; }; // expected-warning {{declaration shadows a local variable}}
     auto f4 = [&] (int param) { ; }; // expected-warning {{declaration shadows a local variable}}
+#endif
+  }
+
+  // Warn for variables that are implicitly captured.
+  {
+    auto f1 = [=] () {
+      {
+        int var = 1; // expected-warning {{declaration shadows a local variable}}
+      }
+      int x = var; // expected-note {{variable 'var' is captured here}}
+    };
+    auto f2 = [&]
+#ifdef AVOID
+      (int param) {
+#else
+      (int param) { // expected-warning {{declaration shadows a local variable}}
+#endif
+      int x = var; // expected-note {{variable 'var' is captured here}}
+      int var = param; // expected-warning {{declaration shadows a local variable}}
+    };
+  }
+
+  // Warn for variables that are explicitly captured when a lambda has a default
+  // capture specifier.
+  {
+    auto f1 = [=, &var] () { // expected-note {{variable 'var' is captured here}}
+      int x = param; // expected-note {{variable 'param' is captured here}}
+      int var = 0; // expected-warning {{declaration shadows a local variable}}
+      int param = 0; // expected-warning {{declaration shadows a local variable}}
+    };
   }
 
   // Warn normally inside of lambdas.
   auto l1 = [] { // expected-note {{previous declaration is here}}
       int x = 1; // expected-note {{previous declaration is here}}
       { int x = 2; } // expected-warning {{declaration shadows a local variable}}
   };
   auto l2 = [] (int x) { // expected-note {{previous declaration is here}}

 #ifdef AVOID
       int var = 1; // no warning
 #else
       int var = 1; // expected-warning {{declaration shadows a local variable}}
 #endif
     };
 #ifdef AVOID
     auto f1 = [] { int var = 1; }; // no warning
+    auto f2 = [=] { int var = 1; }; // no warning
 #else
     auto f1 = [] { int var = 1; }; // expected-warning {{declaration shadows a local variable}}
-#endif
     auto f2 = [=] { int var = 1; }; // expected-warning {{declaration shadows a local variable}}
+#endif
     auto f3 = [var] // expected-note {{variable 'var' is explicitly captured here}}
       { int var = 1; }; // expected-warning {{declaration shadows a local variable}}
+    auto f4 = [&] {
+      int x = var; // expected-note {{variable 'var' is captured here}}
+      int var = 2; // expected-warning {{declaration shadows a local variable}}
+    };
+  };
+  auto l6 = [&] {
+    auto f1 = [param] { // expected-note {{variable 'param' is explicitly captured here}}
+      int param = 0; // expected-warning {{declaration shadows a local variable}}
+    };
   };
 
   // Generic lambda arguments should work.
 #ifdef AVOID
   auto g1 = [](auto param) { ; }; // no warning
+  auto g2 = [=](auto param) { ; }; // no warning
 #else
   auto g1 = [](auto param) { ; }; // expected-warning {{declaration shadows a local variable}}
+  auto g2 = [=](auto param) { ; }; // expected-warning {{declaration shadows a local variable}}
 #endif
-  auto g2 = [param] // expected-note {{variable 'param' is explicitly captured here}}
+  auto g3 = [param] // expected-note {{variable 'param' is explicitly captured here}}
    (auto param) { ; }; // expected-warning {{declaration shadows a local variable}}
 }