<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Mar 27, 2020 at 5:16 PM Arthur O'Dwyer via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">Richard: Okay, filed <a href="https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94376" target="_blank">https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94376</a> !<div><br></div><div>David: You are correct, the bug in function_ref appeared only when constructing from `const function_ref&&`. When I said that the constructor template suppressed the copy constructor, I was wrong. The implicitly generated copy constructor is still there, and it's the best match for `const function_ref&` but not for `const function_ref&&`.  The bug would also appear with construction from `volatile function_ref&`, but, let's not go there. :)</div><div><br></div><div>IMHO (YMMV), it would be worth <i>additionally</i> adding a complex but "realistic" test case that depends on the GCC bug, as opposed to your own test's simple but "unrealistic" cast to `const function_ref&&`. Here's a test case that I believe would have dereferenced null on GCC but would have worked fine on Clang—</div><div><br></div><div>





<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:16px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures">TEST(FunctionRefTest, BadCopyGCCBug) {</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:16px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures"><span>  </span>auto A = [] { return 1; };</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:16px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures"><span>  </span>function_ref<int()> W = A;</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:16px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures"><span>  </span>auto LW = [=]() {  // captures a data member 'const function_ref W'</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:16px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures"><span>    </span>return [=]() {</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:16px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures"><span>      </span>return W();</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:16px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures"><span>    </span>};</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:16px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures"><span>  </span>}();</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:16px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures"><span>  </span>auto LX = std::move(LW);  // calls function_ref's 'const&&' constructor</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:16px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures"><span>  </span>W = nullptr;</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:16px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures"><span>  </span>ASSERT_EQ(LX(), 1);</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:16px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures">}</span></p></div></div></div></blockquote><div><br></div><div>Yeah, I tend to err on the side of fairly isolated testing. I guess a fairly closer-to-practical, though smaller (in terms of feature interactions, not necessarily in lines of code) example might be to std::move on a const ref, rather than the raw cast - the next step beyond that would be to use a template to make it more realistic as to why you'd be moving a const ref, etc.<br><br>But I think the narrower test is alright.<br><br>- Dave</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div><br></div><div>–Arthur</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Mar 27, 2020 at 7:48 PM Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Thu, 26 Mar 2020 at 21:50, Arthur O'Dwyer via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:</div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div class="gmail_quote"><div>[...]</div><div>Argh! That is insanely sneaky, and it is quite probable IMHO that this is a core-language bug in GCC, because GCC behaves differently from EDG and Clang here.</div><div></div><div><a href="https://godbolt.org/z/oCvLpv" target="_blank">https://godbolt.org/z/oCvLpv</a><br></div><div>On GCC, when you have a lambda nested within a lambda, and you capture-by-copy a variable which was captured-by-copy using [=] or [i] (but not C++14 init-capture [i=i]), then your data member for that capture will have type `const I`, not just `I`.</div><div>On Clang and EDG, [=], [i], and [i=i] all behave equivalently, and capture a data member with type `I`.</div><div><br></div><div>I don't see anything about this on <a href="https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=lambda%20capture%20const" target="_blank">https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=lambda%20capture%20const</a> — Richard, you might be best able to describe the issue correctly ;) but if you want me to file it, I will.  (Might be a corollary of <a href="https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86697" target="_blank">bug 86697</a>.)</div></div></div></div></div></div></div></div></blockquote><div><br></div><div>Oh wow, so this would only fail with a combination of libstdc++ (which makes a copy of the lambda and destroy the original before copying the lambda) and GCC (which has a bug in how it determines the type of a nested capture)? Fascinating! :)</div><div><br></div><div>[expr.prim.lambda.capture]p10 is the relevant rule: "The type of such a data member is the referenced type if the entity is a reference to an object, an lvalue reference to the referenced function type if the entity is a reference to a function, or the type of the corresponding captured entity otherwise."</div><div><br></div><div>Regardless of whether you think the captured entity is the original variable or the member of the outer closure type, the type of that entity is not const-qualified. So the inner capture should not have a const-qualified type.</div><div><br></div><div>Given that you identified the GCC capture bug, I think you should do the honors :)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div class="gmail_quote"><div>Regardless, llvm::function_ref's SFINAE should still be fixed. This is a bug in llvm::function_ref, exposed by a bug in GCC. (Or vice versa.)</div></div></div></div></div></div></div></div></blockquote><div><br></div><div>Yup. </div></div></div>
</blockquote></div></div>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div>