[PATCH] D29748: [cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.
faisalv closed this revision. faisalv added a comment. Closed by commit https://reviews.llvm.org/rL295279 https://reviews.llvm.org/D29748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29748: [cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.
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 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 }() == // 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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29748: [cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.
hubert.reinterpretcast added inline comments.
Comment at: lib/AST/ExprConstant.cpp:5061
+ APValue RVal;
+ // FIXME: We need to make sure we're passing the right type that
+ // maintains cv-qualifiers.
faisalv wrote:
> rsmith wrote:
> > faisalv wrote:
> > > 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?
> > 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 }(); // should be accepted
> > }
> > ```
> OK - but how is the address of a local variable a constant expression?
I guess this one is safer:
```
void f() {
volatile int n;
constexpr volatile int *p = [&]{ return false ? : nullptr; }();
}
```
https://reviews.llvm.org/D29748
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29748: [cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.
faisalv added inline comments.
Comment at: lib/AST/ExprConstant.cpp:5061
+ APValue RVal;
+ // FIXME: We need to make sure we're passing the right type that
+ // maintains cv-qualifiers.
rsmith wrote:
> faisalv wrote:
> > 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?
> 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 }(); // should be accepted
> }
> ```
OK - but how is the address of a local variable a constant expression?
https://reviews.llvm.org/D29748
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29748: [cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.
faisalv updated this revision to Diff 87943.
faisalv marked 6 inline comments as done.
faisalv added a comment.
Incorporated Richard's feedback and added comments.
https://reviews.llvm.org/D29748
Files:
lib/AST/ExprConstant.cpp
test/SemaCXX/cxx1z-constexpr-lambdas.cpp
Index: test/SemaCXX/cxx1z-constexpr-lambdas.cpp
===
--- test/SemaCXX/cxx1z-constexpr-lambdas.cpp
+++ test/SemaCXX/cxx1z-constexpr-lambdas.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks %s
-// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fdelayed-template-parsing %s
-// RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -fblocks %s -DCPP14_AND_EARLIER
+// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks %s -fcxx-exceptions
+// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fdelayed-template-parsing %s -fcxx-exceptions
+// RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -fblocks %s -DCPP14_AND_EARLIER -fcxx-exceptions
namespace test_lambda_is_literal {
@@ -157,18 +157,111 @@
} // end ns1_simple_lambda
-namespace ns1_unimplemented {
-namespace ns1_captures {
+namespace test_captures_1 {
+namespace ns1 {
constexpr auto f(int i) {
- double d = 3.14;
- auto L = [=](auto a) { //expected-note{{coming soon}}
-int Isz = i + d;
-return sizeof(i) + sizeof(a) + sizeof(d);
+ struct S { int x; } s = { i * 2 };
+ auto L = [=](auto a) {
+return i + s.x + a;
};
return L;
}
-constexpr auto M = f(3); //expected-error{{constant expression}} expected-note{{in call to}}
-} // end ns1_captures
+constexpr auto M = f(3);
+
+static_assert(M(10) == 19);
+
+} // end test_captures_1::ns1
+
+namespace ns2 {
+
+constexpr auto foo(int n) {
+ auto L = [i = n] (auto N) mutable {
+if (!N(i)) throw "error";
+return [] {
+ return ++i;
+};
+ };
+ auto M = L([n](int p) { return p == n; });
+ M(); M();
+ L([n](int p) { return p == n + 2; });
+
+ return L;
+}
+
+constexpr auto L = foo(3);
+
+} // end test_captures_1::ns2
+namespace ns3 {
+
+constexpr auto foo(int n) {
+ auto L = [i = n] (auto N) mutable {
+if (!N(i)) throw "error";
+return [] {
+ return [i]() mutable {
+return ++i;
+ };
+};
+ };
+ auto M = L([n](int p) { return p == n; });
+ M()(); M()();
+ L([n](int p) { return p == n; });
+
+ return L;
+}
+
+constexpr auto L = foo(3);
+} // end test_captures_1::ns3
+
+namespace ns2_capture_this_byval {
+struct S {
+ int s;
+ constexpr S(int s) : s{s} { }
+ constexpr auto f(S o) {
+return [*this,o] (auto a) { return s + o.s + a.s; };
+ }
+};
+
+constexpr auto L = S{5}.f(S{10});
+static_assert(L(S{100}) == 115);
+} // end test_captures_1::ns2_capture_this_byval
+
+namespace ns2_capture_this_byref {
+
+struct S {
+ int s;
+ constexpr S(int s) : s{s} { }
+ constexpr auto f() const {
+return [this] { return s; };
+ }
+};
+
+constexpr S SObj{5};
+constexpr auto L = SObj.f();
+constexpr int I = L();
+static_assert(I == 5);
+
+} // end ns2_capture_this_byref
+
+} // end test_captures_1
+
+namespace test_capture_array {
+namespace ns1 {
+constexpr auto f(int I) {
+ int arr[] = { I, I *2, I * 3 };
+ auto L1 = [&] (auto a) { return arr[a]; };
+ int r = L1(2);
+ struct X { int x, y; };
+ return [=](auto a) { return X{arr[a],r}; };
+}
+constexpr auto L = f(3);
+static_assert(L(0).x == 3);
+static_assert(L(0).y == 9);
+static_assert(L(1).x == 6);
+static_assert(L(1).y == 9);
+} // end ns1
+
+} // end test_capture_array
+namespace ns1_unimplemented {
} // end ns1_unimplemented
} // end ns test_lambda_is_cce
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -425,6 +425,9 @@
/// Index - The call index of this call.
unsigned Index;
+llvm::DenseMap LambdaCaptureFields;
+FieldDecl *LambdaThisCaptureField;
+
CallStackFrame(EvalInfo , SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
APValue *Arguments);
@@ -2279,6 +2282,10 @@
return true;
}
+static bool handleLValueToRValueConversion(EvalInfo , const Expr *Conv,
+ QualType Type, const LValue ,
+ APValue );
+
/// Try to evaluate the initializer for a variable declaration.
///
/// \param Info Information about the ongoing evaluation.
@@ -2290,6 +2297,7 @@
static bool evaluateVarDeclInit(EvalInfo , const Expr *E,
const VarDecl *VD, CallStackFrame *Frame,
APValue *) {
+
// If this is a parameter to an active constexpr function call, perform
// argument substitution.
if (const ParmVarDecl *PVD = dyn_cast(VD)) {
@@ -4180,6 +4188,10 @@
return false;
This->moveInto(Result);
return true;
+ } else if (MD &&
[PATCH] D29748: [cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.
rsmith added inline comments.
Comment at: lib/AST/ExprConstant.cpp:5061
+ APValue RVal;
+ // FIXME: We need to make sure we're passing the right type that
+ // maintains cv-qualifiers.
faisalv wrote:
> 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?
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 }(); // should be accepted
}
```
Comment at: lib/AST/ExprConstant.cpp:5066-5068
+ assert(RVal.isLValue() && "Reference captures through their "
+"corresponding field members must refer to
"
+"lvalues (VarDecls or FieldDecls)");
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?
Comment at: lib/AST/ExprConstant.cpp:5473-5474
+
+ if (HandleLValueMember(Info, E, Result,
+ Info.CurrentCall->LambdaThisCaptureField)) {
+if (Info.CurrentCall->LambdaThisCaptureField->getType()
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.
Comment at: lib/AST/ExprConstant.cpp:6338-6339
+// occurred.
+if (!CurFieldInit)
+ return false;
+
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.
Repository:
rL LLVM
https://reviews.llvm.org/D29748
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29748: [cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.
faisalv added inline comments. Comment at: lib/AST/ExprConstant.cpp:5061 + APValue RVal; + // FIXME: We need to make sure we're passing the right type that + // maintains cv-qualifiers. 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? Comment at: lib/AST/ExprConstant.cpp:5486 + } + //Info.FFDiag(E); + return false; I need to delete this comment... Repository: rL LLVM https://reviews.llvm.org/D29748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29748: [cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.
faisalv created this revision.
faisalv added a project: clang-c.
Herald added a subscriber: EricWF.
This patch attempts to enable evaluation of all forms of captures (however
deeply nested) within constexpr lambdas.
Appreciate the feedback.
Thanks!
Repository:
rL LLVM
https://reviews.llvm.org/D29748
Files:
lib/AST/ExprConstant.cpp
test/SemaCXX/cxx1z-constexpr-lambdas.cpp
Index: test/SemaCXX/cxx1z-constexpr-lambdas.cpp
===
--- test/SemaCXX/cxx1z-constexpr-lambdas.cpp
+++ test/SemaCXX/cxx1z-constexpr-lambdas.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks %s
-// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fdelayed-template-parsing %s
-// RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -fblocks %s -DCPP14_AND_EARLIER
+// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks %s -fcxx-exceptions
+// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fdelayed-template-parsing %s -fcxx-exceptions
+// RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -fblocks %s -DCPP14_AND_EARLIER -fcxx-exceptions
namespace test_lambda_is_literal {
@@ -157,18 +157,111 @@
} // end ns1_simple_lambda
-namespace ns1_unimplemented {
-namespace ns1_captures {
+namespace test_captures_1 {
+namespace ns1 {
constexpr auto f(int i) {
- double d = 3.14;
- auto L = [=](auto a) { //expected-note{{coming soon}}
-int Isz = i + d;
-return sizeof(i) + sizeof(a) + sizeof(d);
+ struct S { int x; } s = { i * 2 };
+ auto L = [=](auto a) {
+return i + s.x + a;
};
return L;
}
-constexpr auto M = f(3); //expected-error{{constant expression}} expected-note{{in call to}}
-} // end ns1_captures
+constexpr auto M = f(3);
+
+static_assert(M(10) == 19);
+
+} // end test_captures_1::ns1
+
+namespace ns2 {
+
+constexpr auto foo(int n) {
+ auto L = [i = n] (auto N) mutable {
+if (!N(i)) throw "error";
+return [] {
+ return ++i;
+};
+ };
+ auto M = L([n](int p) { return p == n; });
+ M(); M();
+ L([n](int p) { return p == n + 2; });
+
+ return L;
+}
+
+constexpr auto L = foo(3);
+
+} // end test_captures_1::ns2
+namespace ns3 {
+
+constexpr auto foo(int n) {
+ auto L = [i = n] (auto N) mutable {
+if (!N(i)) throw "error";
+return [] {
+ return [i]() mutable {
+return ++i;
+ };
+};
+ };
+ auto M = L([n](int p) { return p == n; });
+ M()(); M()();
+ L([n](int p) { return p == n; });
+
+ return L;
+}
+
+constexpr auto L = foo(3);
+} // end test_captures_1::ns3
+
+namespace ns2_capture_this_byval {
+struct S {
+ int s;
+ constexpr S(int s) : s{s} { }
+ constexpr auto f(S o) {
+return [*this,o] (auto a) { return s + o.s + a.s; };
+ }
+};
+
+constexpr auto L = S{5}.f(S{10});
+static_assert(L(S{100}) == 115);
+} // end test_captures_1::ns2_capture_this_byval
+
+namespace ns2_capture_this_byref {
+
+struct S {
+ int s;
+ constexpr S(int s) : s{s} { }
+ constexpr auto f() const {
+return [this] { return s; };
+ }
+};
+
+constexpr S SObj{5};
+constexpr auto L = SObj.f();
+constexpr int I = L();
+static_assert(I == 5);
+
+} // end ns2_capture_this_byref
+
+} // end test_captures_1
+
+namespace test_capture_array {
+namespace ns1 {
+constexpr auto f(int I) {
+ int arr[] = { I, I *2, I * 3 };
+ auto L1 = [&] (auto a) { return arr[a]; };
+ int r = L1(2);
+ struct X { int x, y; };
+ return [=](auto a) { return X{arr[a],r}; };
+}
+constexpr auto L = f(3);
+static_assert(L(0).x == 3);
+static_assert(L(0).y == 9);
+static_assert(L(1).x == 6);
+static_assert(L(1).y == 9);
+} // end ns1
+
+} // end test_capture_array
+namespace ns1_unimplemented {
} // end ns1_unimplemented
} // end ns test_lambda_is_cce
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -425,6 +425,9 @@
/// Index - The call index of this call.
unsigned Index;
+llvm::DenseMap LambdaCaptureFields;
+FieldDecl *LambdaThisCaptureField;
+
CallStackFrame(EvalInfo , SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
APValue *Arguments);
@@ -2279,6 +2282,10 @@
return true;
}
+static bool handleLValueToRValueConversion(EvalInfo , const Expr *Conv,
+ QualType Type, const LValue ,
+ APValue );
+
/// Try to evaluate the initializer for a variable declaration.
///
/// \param Info Information about the ongoing evaluation.
@@ -2290,6 +2297,7 @@
static bool evaluateVarDeclInit(EvalInfo , const Expr *E,
const VarDecl *VD, CallStackFrame *Frame,
APValue *) {
+
// If this is a parameter to an active constexpr function call, perform
// argument substitution.
if (const ParmVarDecl *PVD = dyn_cast(VD)) {
@@
