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

[Zachary Turner]

We don't have the wheel though.  For example, it's not implemented on
Android and it's not implemented on Windows.  I could duplicate a bunch of
code from LLVM, but why?  The code is already there in LLVM.  And what
about the Android people?  There's currently about 3 completely different
implementations of backtracing (MacOSX, FreeBSD, Linux).  All of them print
backtraces in a different format.  We can call 1 function and have
backtraces in the same format for everyone, including Windows and Android,
right now.  This seems like kind of a straightforward case of code reuse to
me, and a clear win, so I'm not sure why it's that contentious.

If we need to interoperate with StreamString, then we should implement an
overload **in LLVM** that returns an std::string.

On Wed, Mar 4, 2015 at 5:30 PM Enrico Granata <egranata at apple.com> wrote:

> 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>
> 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
>>
>> EMAIL PREFERENCES
>>   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/20150305/0cfbacfd/attachment.html>