[PATCH] D29748: [cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.

[Richard Smith via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lib/AST/ExprConstant.cpp:428-429
 
+    llvm::DenseMap<const VarDecl *, FieldDecl *> LambdaCaptureFields;
+    FieldDecl *LambdaThisCaptureField;
+
----------------
I'm a little concerned that adding this to every `CallStackFrame` may have a nontrivial impact on the overall stack usage of deeply-recursing constexpr evaluataions. (I'd also like to cache this map rather than recomputing it repeatedly.) But let's try this and see how it goes; we can look into caching the map as a later change.


================
Comment at: lib/AST/ExprConstant.cpp:4194
+    MD->getParent()->getCaptureFields(Frame.LambdaCaptureFields,
+      Frame.LambdaThisCaptureField);
   }
----------------
Indent.


================
Comment at: lib/AST/ExprConstant.cpp:5061
+      // ... then update it to refer to the field of the closure object
+      // that represents the capture.
+      if (!HandleLValueMember(Info, E, Result, FD))
----------------
```constexpr bool b = [&]{ return &n; }() == &n; // should be accepted```

... is more what I was thinking.


================
Comment at: lib/AST/ExprConstant.cpp:5067-5071
+          APValue RVal;
+          if (!handleLValueToRValueConversion(Info, E, FD->getType(), Result,
+                                              RVal))
+            return false;
+          Result.setFrom(Info.Ctx, RVal);
----------------
Too much indentation here.


https://reviews.llvm.org/D29748