<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 20, 2015 at 11:52 AM, Adrian Prantl <span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div class="h5"><blockquote type="cite"><div>On Jan 20, 2015, at 11:46 AM, Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>> wrote:</div><br><div><div style="word-wrap:break-word"><br><div><blockquote type="cite"><div>On Jan 20, 2015, at 11:10 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><div><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 20, 2015 at 10:58 AM, Adrian Prantl <span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
> Begin forwarded message:<br>
><br>
> Subject: Re: [llvm] r226588 - Add an assertion and prefer a crash over an infinite loop.<br>
> From: "Duncan P. N. Exon Smith" <<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>><br>
> Date: January 20, 2015 at 10:53:38 AM PST<br>
> Cc: <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> To: Adrian Prantl <<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>><br>
<div><div>><br>
><br>
>> On 2015 Jan 20, at 10:03, Adrian Prantl <<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>> wrote:<br>
>><br>
>> Author: adrian<br>
>> Date: Tue Jan 20 12:03:37 2015<br>
>> New Revision: 226588<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=226588&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=226588&view=rev</a><br>
>> Log:<br>
>> Add an assertion and prefer a crash over an infinite loop.<br>
>><br>
>> Modified:<br>
>> llvm/trunk/lib/IR/DebugInfo.cpp<br>
>><br>
>> Modified: llvm/trunk/lib/IR/DebugInfo.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=226588&r1=226587&r2=226588&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=226588&r1=226587&r2=226588&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/IR/DebugInfo.cpp (original)<br>
>> +++ llvm/trunk/lib/IR/DebugInfo.cpp Tue Jan 20 12:03:37 2015<br>
>> @@ -527,12 +527,15 @@ bool DISubprogram::Verify() const {<br>
>> while ((IA = DL.getInlinedAt()))<br>
>> DL = DebugLoc::getFromDILocation(IA);<br>
>> DL.getScopeAndInlinedAt(Scope, IA);<br>
>> + assert(Scope && "debug location has no scope");<br>
>> assert(!IA);<br>
>> while (!DIDescriptor(Scope).isSubprogram()) {<br>
>> DILexicalBlockFile D(Scope);<br>
>> Scope = D.isLexicalBlockFile()<br>
>> ? D.getScope()<br>
>> : DebugLoc::getFromDILexicalBlock(Scope).getScope();<br>
>> + if (!Scope)<br>
>> + llvm_unreachable("lexical block file has no scope");<br>
><br>
> It's strange that this isn't simply an assertion:<br>
><br>
> assert(Scope && "lexical block file has no scope");<br>
><br>
> But I guess that's so that it crashes even when NDEBUG? (Is that<br>
> reliable?)<br>
<br>
</div></div>Yes, that’s the intention (and llvm_unreachable is meant to be used that way).<br></blockquote><div><br>I don't believe that's the intention of the LLVM style guide - generally we try to remove conditionals like this and replace the conditional with the assertion so the dead code isn't exercised in release builds.<br></div></div></div></div></div></blockquote><div><br></div><div>Isn’t the point of “unreachable” to let the compiler do whatever it wants in this case?</div><div>I thought LLVM was free to just drop the test entirely in release mode?</div><div><br></div></div></div></div></blockquote><div><br></div></div></div><div>After going back to ErrorHandling.h it looks as if report_fatal_error() would be more appropriate.</div></div></div></blockquote><div><br>Yeah, I've never been clear on the rules of report_fatal_error (though it seems problematic for a library like LLVM to just bring down the process, as I understand it) - if returning false and testing works as Duncan suggested, it seems like the best option to me.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div>llvm_unreachable() either becomes llvm_unreachable_internal() which invokes abort() and prints the message or __builtin_unreachable() which indeed drops the message.</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>-- adrian</div></font></span><span class=""><br><blockquote type="cite"><div><div style="word-wrap:break-word"><div><div>Mehdi</div><div><br></div><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span>><br>
> If that's the reason, you should probably add a comment to that<br>
> effect since otherwise I'm likely to switch it to an assert next<br>
> time I look at it ;).<br>
><br>
> (Also, do you know when this situation arises?)<br>
<br>
</span>Happened to me when I compiled an old .ll file without the MDLocation upgrade.<br>
<span><font color="#888888"><br>
-- adrian<br>
</font></span><div><div><br>
><br>
>> }<br>
>> if (!DISubprogram(Scope).describes(F))<br>
>> return false;<br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>
_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></div></blockquote></div><br></div></div></blockquote></span></div><br></div></blockquote></div><br></div></div>