rsmith added a comment.
> But not here, because we would have to verify that the pointer in lam wasn't
> mutated in a previous call of the lambda:
Isn't that guaranteed, because the lambda is not marked `mutable`?
================
Comment at: lib/Sema/SemaChecking.cpp:6594
@@ -6590,1 +6593,3 @@
+ if (LambdaDecl->isLambda())
+ stackE = EvalVal(RetValExp, refVars, /*ParentDecl=*/nullptr);
}
----------------
I don't think it makes sense to use `EvalVal` for this. `EvalVal` is intended
to handle the case of a glvalue that is being returned from the function, which
isn't quite the same thing you want to check here -- I think you would be
better off instead directly checking the captures of the lambda from here
(`LambdaDecl` can give you a list) rather than calling this.
It also seems like the `RetValExp` doesn't actually matter at all here in most
cases. If the closure type captures a local by reference, we should warn,
regardless of the initialization expression, and likewise if it's not `mutable`
and captures a local by address through an init-capture. For `mutable` lambdas
with init-captures that capture by address, perhaps we could do a quick
localized walk here on the `RetValExp` to see if it's returned directly.
================
Comment at: lib/Sema/SemaChecking.cpp:6845
@@ -6832,3 +6844,3 @@
const Decl *ParentDecl) {
- do {
+ for (;;) {
// We should only be called for evaluating non-pointer expressions, or
----------------
We prefer `while (true)` over `for (;;)` (but I agree the existing `do ...
while (true);` is terrible).
================
Comment at: lib/Sema/SemaChecking.cpp:6886-6887
@@ -6873,3 +6885,4 @@
if (V->hasLocalStorage()) {
- if (!V->getType()->isReferenceType())
+ auto *RD = V->getType()->getAsCXXRecordDecl();
+ if (!V->getType()->isReferenceType() && !(RD && RD->isLambda()))
return DR;
----------------
This is the kind of problem I was worried about. Given:
auto &f() {
auto lam = [] {};
return lam;
}
... we'll currently diagnose that we're returning the address of a local, from
here. But with this change applied, we lose that diagnostic because `EvalVal`
can't tell the difference between this case (returning by reference) and
returning the lambda by value.
================
Comment at: test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp:185
@@ -184,3 +184,3 @@
f(y, d);
- f(z, d);
+ f(z, d); // expected-warning {{address of stack memory associated
with local variable 'z' returned}}
decltype(a) A = a;
----------------
This diagnostic looks like it'll be confusing. Can you add a note pointing at
the `return` statement in question (we're warning about the one on line 179, I
assume)?
================
Comment at: test/SemaCXX/cxx1y-init-captures.cpp:123-124
@@ -122,3 +122,4 @@
};
- return M;
+ // FIXME: We shouldn't emit this warning here.
+ return M; // expected-warning{{stack memory}}
};
----------------
Do you know why this happens? It looks like `EvalVal` should bail out on the
`DeclRefExpr` in the init-capture, because it
`refersToEnclosingVariableOrCapture`.
================
Comment at: test/SemaCXX/return-lambda-stack-addr.cpp:68-72
@@ +67,7 @@
+
+auto ref_ref() {
+ int x;
+ int &y = x; // expected-note{{binding reference variable 'y'
here}}
+ return [&] { (void)y; }; // expected-warning{{address of stack memory
associated with local variable 'x' returned}}
+}
+
----------------
I'd like to see another testcase for the case when a local reference is safely
captured by reference:
auto ref_ref_ok(int &r) {
int &y = r;
return [&] { return y; }; // no warning
}
https://reviews.llvm.org/D24639
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
