<div dir="ltr"><div dir="ltr">On Sun, Mar 22, 2020 at 1:48 PM David Blaikie via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></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">On Sun, Mar 22, 2020 at 10:40 AM Johannes Doerfert <<a href="mailto:johannes@jdoerfert.de" target="_blank">johannes@jdoerfert.de</a>> wrote:<br></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">Some buildbots, I think only Windows buildbots for some reason, crashed <br>
in this function.<br>
<br>
The reason, as described, is that an `llvm::function_ref` cannot be <br>
copied and then reused. It just happened to work on almost all <br>
configurations.<br></blockquote><div><br>That doesn't seem to be accurate, or if it is there's a lot of mistakes in the codebase - basically every function_ref parameter I can see in LLVM is passing by value, not by const ref. The implementation of llvm::function_ref looks quite copyable so long as it doesn't outlive the functor it is pointing to.<br></div></div></div></blockquote><div><br></div><div>David is correct. llvm::function_ref, like std::reference_wrapper, is a trivially copyable type, and it's designed to be copied.</div><div>Like string_view and reference_wrapper, function_ref is designed to be passed by value. Redundantly passing function_ref <i>again by reference</i> is a code smell.</div><div><br></div><div>I've also checked the code here, and it looks like there are only two callers of `anyScoreOrCondition` — both in Sema/SemaOpenMP.cpp — and they are both fine. FWIW, I would recommend reverting Johannes' change and seeing if those Windows buildbots are still unhappy (and if so, why).</div><div><br></div><div><br></div><div>Btw, one failure mode I'm aware of, but which I believe is NOT relevant in Johannes' case, is that `function_ref`'s converting constructor behaves differently from its copy constructor.</div><div><br></div><div>
<p class="gmail-p1" style="margin:0px;font:16px Menlo;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures">int main()</span></p><p class="gmail-p1" style="margin:0px;font:16px Menlo;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures">{</span></p><p class="gmail-p1" style="margin:0px;font:16px Menlo;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><span class="gmail-Apple-converted-space"> </span>auto lamb = [](){ return 42; };</span></p><p class="gmail-p1" style="margin:0px;font:16px Menlo;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><span class="gmail-Apple-converted-space"> </span>auto sheep = [](){ return 43; };</span></p><p class="gmail-p1" style="margin:0px;font:16px Menlo;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><span class="gmail-Apple-converted-space"> </span>llvm::function_ref<int()> one = lamb;</span></p><p class="gmail-p2" style="margin:0px;font:16px Menlo;color:rgb(0,0,0);min-height:19px"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"></span><br></p><p class="gmail-p1" style="margin:0px;font:16px Menlo;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><span class="gmail-Apple-converted-space"> </span>llvm::function_ref<int()> twoA = one; // twoA refers to lamb</span></p><p class="gmail-p1" style="margin:0px;font:16px Menlo;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><span class="gmail-Apple-converted-space"> </span>llvm::function_ref<short()> twoB = one; // twoB refers to one which refers to lamb</span></p><p class="gmail-p2" style="margin:0px;font:16px Menlo;color:rgb(0,0,0);min-height:19px"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"></span><br></p><p class="gmail-p1" style="margin:0px;font:16px Menlo;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><span class="gmail-Apple-converted-space"> </span>one = sheep;</span></p><p class="gmail-p2" style="margin:0px;font:16px Menlo;color:rgb(0,0,0);min-height:19px"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"></span><br></p><p class="gmail-p1" style="margin:0px;font:16px Menlo;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><span class="gmail-Apple-converted-space"> </span>assert(twoA() == 42); // twoA refers to lamb</span></p><p class="gmail-p1" style="margin:0px;font:16px Menlo;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><span class="gmail-Apple-converted-space"> </span>assert(twoB() == 43); // twoB refers to one which refers to sheep</span></p><p class="gmail-p1" 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)">
</p><p class="gmail-p1" 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 class="gmail-s1" style="font-variant-ligatures:no-common-ligatures">}</span></p></div><div><br></div><div>That is, if you have a function that takes a parameter of type function_ref<A>, and someone passes it an argument of type function_ref<B>, then inside the function your parameter will be referring to that argument itself instead of to its referent.</div><div>However, in Johannes' particular case, we have no function_refs referring to other function_refs. We just make a lambda and take a function_ref to it, the end. So I'm pretty sure this pitfall is irrelevant.</div><div><br></div><div>–Arthur</div></div></div>