<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 28, 2015, at 1:21 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" class="">richard@metafoo.co.uk</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Hi Ben,<br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Tue, Jan 27, 2015 at 3:06 PM, Ben Langmuir <span dir="ltr" class=""><<a href="mailto:blangmuir@apple.com" target="_blank" class="">blangmuir@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Richard,<br class="">
<br class="">
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 ;-)</blockquote><div class=""> </div><div class="">LGTM as a diagnostics improvement (but see below).</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Don't mistakenly capture byref in block nested in lambda<br class="">
<br class="">
Even if an enclosing lambda captures a variable by reference, we capture<br class="">
it by *copy* in a block unless the variable itself was declared with a<br class="">
reference type. Codegen was already correct, but we were not diagnosing<br class="">
attempts to modify the variable in Sema.<br class=""></blockquote><div class=""><br class=""></div><div class="">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.</div></div></div></div></div></blockquote><div><br class=""></div><div>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:</div><div><br class=""></div><div>int x = 0;</div><div>[&]() {</div><div> ^{</div><div> x = 1; // this diagnoses now, but didn’t before</div><div> }();</div><div>}();</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This also fixes a crash on invalid when trying to modify a variable with<br class="">
non-trivial copy constructor and assignment operator in such a<br class="">
situation.<br class=""></blockquote><div class=""><br class=""></div><div class="">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.</div></div></div></div>
</div></blockquote></div><br class=""><div class="">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:</div><div class=""><br class=""></div><div class="">$ cat red.cpp</div><div class="">struct A {<br class=""> A() = default;<br class=""> A(const A &); // non-trivial copy<br class=""> A &operator=(const A &); // non-trivial assign<br class="">};<br class=""><br class="">void test() {<br class=""> A a;<br class=""> [&]() { ^{ (void)a; }(); }();<br class="">}</div><div class=""><br class=""></div><div class="">$ clang++ -std=c++11 -fblocks red.cpp</div><div class=""><br class=""></div><div class="">Ben</div></body></html>