[llvm] r226588 - Add an assertion and prefer a crash over an infinite loop.

[Mehdi Amini]

> On Jan 20, 2015, at 11:10 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Tue, Jan 20, 2015 at 10:58 AM, Adrian Prantl <aprantl at apple.com <mailto:aprantl at apple.com>> wrote:
> 
> 
> > Begin forwarded message:
> >
> > Subject: Re: [llvm] r226588 - Add an assertion and prefer a crash over an infinite loop.
> > From: "Duncan P. N. Exon Smith" <dexonsmith at apple.com <mailto:dexonsmith at apple.com>>
> > Date: January 20, 2015 at 10:53:38 AM PST
> > Cc: llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> > To: Adrian Prantl <aprantl at apple.com <mailto:aprantl at apple.com>>
> >
> >
> >> On 2015 Jan 20, at 10:03, Adrian Prantl <aprantl at apple.com <mailto:aprantl at apple.com>> wrote:
> >>
> >> Author: adrian
> >> Date: Tue Jan 20 12:03:37 2015
> >> New Revision: 226588
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=226588&view=rev <http://llvm.org/viewvc/llvm-project?rev=226588&view=rev>
> >> Log:
> >> Add an assertion and prefer a crash over an infinite loop.
> >>
> >> Modified:
> >>   llvm/trunk/lib/IR/DebugInfo.cpp
> >>
> >> Modified: llvm/trunk/lib/IR/DebugInfo.cpp
> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=226588&r1=226587&r2=226588&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=226588&r1=226587&r2=226588&view=diff>
> >> ==============================================================================
> >> --- llvm/trunk/lib/IR/DebugInfo.cpp (original)
> >> +++ llvm/trunk/lib/IR/DebugInfo.cpp Tue Jan 20 12:03:37 2015
> >> @@ -527,12 +527,15 @@ bool DISubprogram::Verify() const {
> >>        while ((IA = DL.getInlinedAt()))
> >>          DL = DebugLoc::getFromDILocation(IA);
> >>        DL.getScopeAndInlinedAt(Scope, IA);
> >> +        assert(Scope && "debug location has no scope");
> >>        assert(!IA);
> >>        while (!DIDescriptor(Scope).isSubprogram()) {
> >>          DILexicalBlockFile D(Scope);
> >>          Scope = D.isLexicalBlockFile()
> >>                      ? D.getScope()
> >>                      : DebugLoc::getFromDILexicalBlock(Scope).getScope();
> >> +          if (!Scope)
> >> +            llvm_unreachable("lexical block file has no scope");
> >
> > It's strange that this isn't simply an assertion:
> >
> >    assert(Scope && "lexical block file has no scope");
> >
> > But I guess that's so that it crashes even when NDEBUG?  (Is that
> > reliable?)
> 
> Yes, that’s the intention (and llvm_unreachable is meant to be used that way).
> 
> 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.

Isn’t the point of “unreachable” to let the compiler do whatever it wants in this case?
I thought LLVM was free to just drop the test entirely in release mode?

Mehdi


>  
> >
> > If that's the reason, you should probably add a comment to that
> > effect since otherwise I'm likely to switch it to an assert next
> > time I look at it ;).
> >
> > (Also, do you know when this situation arises?)
> 
> Happened to me when I compiled an old .ll file without the MDLocation upgrade.
> 
> -- adrian
> 
> >
> >>        }
> >>        if (!DISubprogram(Scope).describes(F))
> >>          return false;
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
> >
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150120/2b95255c/attachment.html>