[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks for this! Can you also add a release note for the changes?



================
Comment at: clang/include/clang/AST/Decl.h:673
 
+class VarDecl;
+
----------------
This should be moved up to around line 77 or so (and kept in alphabetical order).


================
Comment at: clang/lib/AST/DeclCXX.cpp:3234-3237
+  if (auto *Var = llvm::dyn_cast<VarDecl>(this))
+    return Var;
+  if (auto *BD = llvm::dyn_cast<BindingDecl>(this))
+    return llvm::dyn_cast<VarDecl>(BD->getDecomposedDecl());
----------------
No need to use the fully-qualified name (we weren't using it for the calls to `isa` either).


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:91
 bool CheckDefaultArgumentVisitor::VisitDeclRefExpr(const DeclRefExpr *DRE) {
-  const NamedDecl *Decl = DRE->getDecl();
+  const ValueDecl *Decl = dyn_cast<ValueDecl>(DRE->getDecl());
+
----------------
1) We should rename `Decl` to something that doesn't shadow the name of a type, since we're here.
2) Can `Decl` be null? You're assuming it can't be below with the `isa` call, so this could switch to `cast`.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:18384-18387
+  if (Underlying) {
+    if (Underlying->hasLocalStorage() && Diagnose)
       diagnoseUncapturableValueReferenceOrBinding(S, Loc, Var);
   }
----------------



================
Comment at: clang/lib/Sema/SemaExpr.cpp:19481-19483
+  const bool RefersToEnclosingScope =
+      (SemaRef.CurContext != VD->getDeclContext() &&
+       VD->getDeclContext()->isFunctionOrMethod() && VD->hasLocalStorage());
----------------



================
Comment at: clang/lib/Sema/SemaExpr.cpp:19485
+  if (RefersToEnclosingScope) {
+    LambdaScopeInfo *const LSI =
+        SemaRef.getCurLambda(/*IgnoreNonLambdaCapturingScope=*/true);
----------------



================
Comment at: clang/lib/Sema/SemaExpr.cpp:19489-19492
+      // If a variable could potentially be odr-used, defer marking it so
+      // until we finish analyzing the full expression for any
+      // lvalue-to-rvalue
+      // or discarded value conversions that would obviate odr-use.
----------------
Can re-flow this comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137244/new/

https://reviews.llvm.org/D137244