This is an archive of the discontinued LLVM Phabricator instance.

[cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.
ClosedPublic

Authored by faisalv on Feb 8 2017, 8:59 PM.

Details

Reviewers
rsmith
Summary

This patch attempts to enable evaluation of all forms of captures (however deeply nested) within constexpr lambdas.

Appreciate the feedback.

Thanks!

Diff Detail

Repository
rL LLVM

Event Timeline

faisalv created this revision.Feb 8 2017, 8:59 PM
faisalv added inline comments.Feb 8 2017, 9:05 PM
lib/AST/ExprConstant.cpp
5061

I don't think we need this fixme - the type of the expression should be correct - all other const checks for mutability have already been performed, right?

5486

I need to delete this comment...

rsmith added inline comments.Feb 9 2017, 2:10 PM
lib/AST/ExprConstant.cpp
5061

When using handleLValueToRValueConversion to obtain the lvalue denoted by a reference, the type you pass should be the reference type itself (FD->getType()). This approach will give the wrong answer when using a captured volatile object:

void f() {
  volatile int n;
  constexpr volatile int *p = [&]{ return &n; }(); // should be accepted
}
5066–5068

I don't see why this assert is correct. If we initialize a reference with a (constant-folded) dereferenced integer cast to pointer type, the value will have integer representation. Just remove the assert?

5473–5474

Please use early-exit style (if (!HandleLValueMember(...)) return false;) here to reduce indentation and make it clearer that you only return false if a diagnostic has already been produced.

6338–6339

Returning false without producing a diagnostic (for the VLA case) is not OK. You should at least produce the basic "not a constant expression" note here.

faisalv updated this revision to Diff 87943.Feb 9 2017, 7:16 PM
faisalv marked 6 inline comments as done.

Incorporated Richard's feedback and added comments.

faisalv added inline comments.Feb 9 2017, 7:20 PM
lib/AST/ExprConstant.cpp
5061

OK - but how is the address of a local variable a constant expression?

hubert.reinterpretcast added inline comments.
lib/AST/ExprConstant.cpp
5061

I guess this one is safer:

void f() {
  volatile int n;
  constexpr volatile int *p = [&]{ return false ? &n : nullptr; }();
}
rsmith accepted this revision.Feb 14 2017, 1:01 PM
rsmith added inline comments.
lib/AST/ExprConstant.cpp
428–429

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.

4194

Indent.

5061
constexpr bool b = [&]{ return &n; }() == &n; // should be accepted

... is more what I was thinking.

5067–5071

Too much indentation here.

This revision is now accepted and ready to land.Feb 14 2017, 1:01 PM