[Lldb-commits] [PATCH] Change lldb_assert() to use llvm::sys::PrintBacktrace
zturner at google.com
Wed Mar 4 17:36:04 PST 2015
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>
>> 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
>> EMAIL PREFERENCES
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the lldb-commits