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

[Zachary Turner]

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

> On Mar 4, 2015, at 5:36 PM, Zachary Turner <zturner at google.com> wrote:
>
> 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.
>
>
> Two things worry me about this:
>
> 1) we have a sanctioned LLDB facility that retrieves a host backtrace, but
> we’re saying that it does not work on all platforms, and instead of making
> it work on all platforms, we’re going to use another API, but have no plan
> to either improve or remove the existing LLDB API for this
>
> Given that we have an LLDB API to do this task, we should use it. If it is
> not an API that we can reasonably support on all platforms we care about,
> then we should remove it in favor of one that we can support on all
> platforms we care about. But we should not be ad-hoc choosing to use one or
> another way to do this task
>
I'm more than happy to remove Host::Backtrace in favor of
llvm::sys::PrintBacktrace().  It is called from 3 locations in the
codebase, and I believe I can replace all 3 of them by adding a simple
overload to llvm that takes an std::string by reference.

Having a sanctioned API doesn't mean we can't improve on things, or move to
a different sanctioned API.  This API is not exposed through the public
interface, so we have no guarantee or requirement to maintain
compatibility.  And there is a better one in LLVM.

"Sanctioned" shouldn't mean "now and forever no matter what new
developments arise".  It should mean "this is what we use because it
happens to be the best thing, but if something better comes along, then by
all means, go for it"


>
> 2) the LLDB API interoperates nicely with our Streams, whereas the LLVM
> one only supports FILE*. In general, Streams are a much nicer interface to
> work with than FILE* are.
>
LLVM is open source too, why can't we modify it?  I modify it all the time,
I'm quite confident I could push a change to LLVM that adds
PrintStackTrace(std::string &output).  I believe that should address the
concerns surrounding FILE*.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150305/49bcd026/attachment.html>