[Lldb-commits] [PATCH] Change lldb_assert() to use llvm::sys::PrintBacktrace

[Enrico Granata]

> On Mar 4, 2015, at 5:18 PM, Zachary Turner <zturner at google.com> wrote:
> 
> Hmm, I'm not sure I agree.  Whether it prints to a Stream or directly to stderr is kind of an implementation detail.  Not very important since it just ends up to stdout or stderr anwyay and we don't do anything else with the backtrace except print it and throw it away.  
> 
> llvm already has functionality built in to serve exactly this purpose.  Why shouldn't we use it?  Not only are we sure that it's implemented on all platforms that LLVM supports,

If that is a concern, I posit that we should implement Host::Backtrace() on all platforms

The alternative of course would be to get rid of Host::Backtrace() entirely, and use the similar LLVM facility - but given how our own facility uses Streams instead of FILE*, I don’t think that is actually a good change

> but the format is consistent on all of these platforms, and anyway why reinvent the wheel?

Except in this case we already have the wheel

> 
> On Wed, Mar 4, 2015 at 5:15 PM Enrico Granata <granata.enrico at gmail.com <mailto:granata.enrico at gmail.com>> wrote:
> ================
> Comment at: source/Utility/LLDBAssert.cpp:14
> @@ -13,1 +13,3 @@
> +
> +#include "llvm/Support/Signals.h"
> 
> ----------------
> I would not do this.
> Printing to a Stream is the LLDB way to do this, no reason for switching to this LLVM API
> 
> ================
> Comment at: source/Utility/LLDBAssert.cpp:36
> @@ -37,1 +35,3 @@
> +        llvm::sys::PrintStackTrace(stderr);
> +        fprintf(stderr, "please file a bug report against lldb reporting this failure log, and as many details as possible\n");
>      }
> ----------------
> Printing to stderr is probably a good idea
> But, again, I prefer to stick to the LLDB host layer
> 
> It's probably fine to reimplement Host::Backtrace() in terms of LLVM APIs if it can be done generally and with decent performance, but I don't see much in terms of added value in this change
> 
> http://reviews.llvm.org/D8069 <http://reviews.llvm.org/D8069>
> 
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/ <http://reviews.llvm.org/settings/panel/emailpreferences/>
> 
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150304/6f148e49/attachment.html>