arphaman created this revision.
arphaman added reviewers: rsmith, rnk.
arphaman added a subscriber: cfe-commits.
arphaman set the repository for this revision to rL LLVM.

This patch avoids the -Wshadow warning for variables which shadow variables 
that aren't captured by lambdas with an explicit capture list. It provides an 
additional note that points to location of the explicit capture.

I noticed that we have -Wshadow-all as well, but I wasn't sure how to preserve 
the old behaviour for it (since I don't think I can check if a diagnostic group 
is enabled), or if the old behaviour should be preserved at all. What do you 
think?

This patch fixes PR 21426, but I think that this is a partial fix, since that 
bug doesn't specify that the capture list should be explicit. I plan on working 
on a similar patch for lambdas with a default capture specifier and implicit 
variable captures, but I'm not sure it's 100% needed for them. Do you think 
this kind of change would make sense for implicit captures as well?


Repository:
  rL LLVM

https://reviews.llvm.org/D26278

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  test/SemaCXX/warn-shadow-in-lambdas.cpp

Index: test/SemaCXX/warn-shadow-in-lambdas.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/warn-shadow-in-lambdas.cpp
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -Wshadow %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.
+  {
+    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}}
+  }
+
+  // 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}}
+    { int x = 1; } // expected-warning {{declaration shadows a local variable}}
+  };
+
+  // Avoid warnings for variables that aren't explicitly captured.
+  {
+    auto f1 = [] { int var = 1; }; // no warning
+    auto f2 = [] (int param) { ; }; // no warning
+    auto f3 = [param] () { int var = 1; }; // no warning
+    auto f4 = [var] (int param) { ; }; // no warning
+  };
+
+  // Warn for variables that are explicitly captured.
+  {
+    auto f1 = [var] () { // expected-note {{variable 'var' is explicitly captured here}}
+      int var = 1; // expected-warning {{declaration shadows a local variable}}
+    };
+    auto f2 = [param]   // expected-note {{variable 'param' is explicitly captured here}}
+     (int param) { ; }; // expected-warning {{declaration shadows a local variable}}
+  }
+
+  // Warn for variables defined in the capture list.
+  auto l3 = [z = var] { // expected-note {{previous declaration is here}}
+    int var = 1; // no warning
+    { int z = 1; } // expected-warning {{declaration shadows a local variable}}
+  };
+  auto l4 = [var = param] (int param) { ; }; // no warning
+
+  // Make sure that inner lambdas work as well.
+  auto l5 = [var, l1] { // expected-note {{variable 'l1' is explicitly captured here}}
+    auto l1 = [] { // expected-warning {{declaration shadows a local variable}}
+      int var = 1; // no warning
+    };
+    auto f1 = [] { int var = 1; }; // no warning
+    auto f2 = [=] { int var = 1; }; // expected-warning {{declaration shadows a local variable}}
+    auto f3 = [var] // expected-note {{variable 'var' is explicitly captured here}}
+      { int var = 1; }; // expected-warning {{declaration shadows a local variable}}
+  };
+
+  // Generic lambda arguments should work.
+  auto g1 = [](auto param) { ; }; // no warning
+  auto g2 = [param] // expected-note {{variable 'param' is explicitly captured here}}
+   (auto param) { ; }; // expected-warning {{declaration shadows a local variable}}
+}
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -6582,6 +6582,17 @@
   return OldDC->isFileContext() ? SDK_Global : SDK_Local;
 }
 
+/// Return the location of the capture if the given lambda captures the given
+/// variable \p VD, or an invalid source location otherwise.
+static SourceLocation getCaptureLocation(const LambdaScopeInfo *LSI,
+                                         const VarDecl *VD) {
+  for (const LambdaScopeInfo::Capture &Capture : LSI->Captures) {
+    if (Capture.isVariableCapture() && Capture.getVariable() == VD)
+      return Capture.getLocation();
+  }
+  return SourceLocation();
+}
+
 /// \brief Diagnose variable or built-in function shadowing.  Implements
 /// -Wshadow.
 ///
@@ -6640,6 +6651,21 @@
 
   DeclContext *OldDC = ShadowedDecl->getDeclContext();
 
+  SourceLocation CaptureLoc;
+  if (isa<VarDecl>(ShadowedDecl) && NewDC && isa<CXXMethodDecl>(NewDC)) {
+    if (const auto *RD = dyn_cast<CXXRecordDecl>(NewDC->getParent())) {
+      // 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())
+          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.
@@ -6662,6 +6688,8 @@
     return;
   ShadowedDeclKind Kind = computeShadowedDeclKind(ShadowedDecl, OldDC);
   Diag(R.getNameLoc(), diag::warn_decl_shadow) << Name << Kind << OldDC;
+  if (!CaptureLoc.isInvalid())
+    Diag(CaptureLoc, diag::note_var_explicitly_captured_here) << Name;
   Diag(ShadowedDecl->getLocation(), diag::note_previous_declaration);
 }
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6227,6 +6227,8 @@
   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">;
 
   // C++14 lambda init-captures.
   def warn_cxx11_compat_init_capture : Warning<
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to