[PATCH] Don't mistakenly capture byref in block nested in lambda

[Ben Langmuir]

> On Jan 28, 2015, at 1:21 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 
> Hi Ben,
> 
> On Tue, Jan 27, 2015 at 3:06 PM, Ben Langmuir <blangmuir at apple.com <mailto:blangmuir at apple.com>> wrote:
> Hi Richard,
> 
> Could you take a look at this patch?  I haven’t messed with capturing in a long time and I don’t want to break it ;-)
>  
> LGTM as a diagnostics improvement (but see below).
>  
>     Don't mistakenly capture byref in block nested in lambda
> 
>     Even if an enclosing lambda captures a variable by reference, we capture
>     it by *copy* in a block unless the variable itself was declared with a
>     reference type. Codegen was already correct, but we were not diagnosing
>     attempts to modify the variable in Sema.
> 
> Hmm, I think I'm missing something here -- it looks like we would diagnose it, but we'd use a suboptimal diagnostic: CheckForModifiableLvalue would diagnose as err_typecheck_assign_const in this case.

It wouldn’t be const at all, because we created the (bogus) byref capture.  It occurs to me that my tests don’t make this obvious, so I’ll add:

int x = 0;
[&]() {
  ^{
    x = 1; // this diagnoses now, but didn’t before
   }();
}();

>  
>     This also fixes a crash on invalid when trying to modify a variable with
>     non-trivial copy constructor and assignment operator in such a
>     situation.
> 
> I can't reproduce this crash with current trunk, assuming this is supposed to be covered by the uses of 'A' in test7 in SemaCXX/blocks.cpp.

Ah my mistake, the crash was reading from the value, not writing to it.  I’ll remove that ‘A’ case in the Sema test and add an IRGen test to exercise it.  If you want to reproduce it:

$ cat red.cpp
struct A {
  A() = default;
  A(const A &); // non-trivial copy
  A &operator=(const A &); // non-trivial assign
};

void test() {
  A a;
  [&]() { ^{ (void)a; }(); }();
}

$ clang++ -std=c++11 -fblocks red.cpp

Ben
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150128/e2600e9a/attachment.html>