[Lldb-commits] [lldb] r231310 - Introduce lldbassert(x)

[Zachary Turner]

Right, but I was just talking about that specific instance of replacing
Host::Backtrace() with llvm::sys::PrintStackTrace().  There is not a good
reason for that specific instance of Host::Backtrace to remain as-is
instead of being replaced with llvm::sys::PrintStackTrace.  Even
independently of whether we delete Host::Backtrace entirely in favor of
llvm::sys::PrintStacktrace across the rest of the codebase.  There's
nothing particualrly useful or special about having a StreamString at that
call-site.

As for the larger issue of whether to replace Host::Backtrace across the
codebase, llvm is open source and has an easy to work with community.  I'm
always surprised when I see things like "the right method signature isn't
available, let's go re-implement hundreds of lines of code on 4 different
platforms".  All we need to do is add the method with the right
signature.  I've
already done that <http://reviews.llvm.org/D8074> and it's been committed
<http://llvm.org/viewvc/llvm-project?revision=231392&view=revision>. It
would be great for the community if people were more open to and
comfortable with working in both codebases.

It's true that we can't add an signature that takes a StreamString&, but
that's the cost of adding unnecessary abstractions (for reference:
llvm::raw_ostream supports printf style formatting, and everything else
that StreamString supports.  I'm still at a loss for why we don't use it,
but that's a discussion for another day).  Because we're stuck in this boat
though doesn't mean we should continue adding unnecessary technical debt.

By making this simple replacement all stack traces will now have file and
line number information, and be able to unwind through inlined functions.

We should always be using llvm library functionality where it's available.
And as Chandler said, not because anyone thinks the LLVM side is perfect,
but  because we are all part of the same project, and we need to benefit
from each other *as well as* share each others' costs.


On Thu, Mar 5, 2015 at 11:37 AM <jingham at apple.com> wrote:

> There are other use cases (e.g. for adding a backtrace to log messages)
> where we want to put the backtrace out in ways lldb is used to outputting
> information - e.g. lldb::Stream's.  So I don't think it is true that "all
> we want to do is get a backtrace to the screen."
>
> Jim
>
> > On Mar 4, 2015, at 5:22 PM, Zachary Turner <zturner at google.com> wrote:
> >
> > As mentioned in the other thread, I think the StreamString thing is kind
> of irrelevant.  The goal here is to get the backtrace to the screen.  I
> would argue the reverse, that we should only be reinventing facilities in
> LLDB when there's a good reason to do so.  In this case, I don't think
> being able to use a StreamString is a compelling enough reason.  On the
> other hand, having a consistent backtrace format which supports all
> platforms is definitely a compelling reason to use the LLVM facility.  So I
> do think it makes sense here.
> >
> > On Wed, Mar 4, 2015 at 5:12 PM Enrico Granata <egranata at apple.com>
> wrote:
> >> On Mar 4, 2015, at 4:39 PM, Zachary Turner <zturner at google.com> wrote:
> >>
> >> BTW, I have just uploaded http://reviews.llvm.org/D8068 to LLVM which
> implements backtracing of self on Windows (previously only backtracing of
> other threads was supported).  So once that goes through, if we switch this
> code to using llvm::sys::PrintBacktrace(),
> >
> > I don’t think we should make this change.
> > Host::Backtrace prints to an lldb_private::Stream which is our preferred
> API for accumulating output - the LLVM version of this seems to print to a
> FILE* which is much less general
> >
> > I understand using LLVM facilities where it makes sense, but we should
> not be doing so blindly when the net effect is a loss of functionality for
> us
> >
> > If you can implement Host::Backtrace in general on all platforms via
> LLVM, feel free to do so - but we should still go through lldb's
> Host::Backtrace for this, and Host::Backtrace should still use our Streams
> >
> >> we should have a standard backtrace format across all platforms that
> LLVM supports.
> >>
> >> I like this feature now that I understand the use case, thanks for
> introducing it.
> >>
> >
> > We had a power outage here in Cupertino, which delayed my reply
> > Glad to hear we all agree on this :-)
> >
> >> On Wed, Mar 4, 2015 at 3:31 PM Siva Chandra <sivachandra at google.com>
> wrote:
> >> On Wed, Mar 4, 2015 at 2:59 PM, Enrico Granata <egranata at apple.com>
> wrote:
> >> > +#ifdef LLDB_CONFIGURATION_DEBUG
> >> > +#define lldbassert(x) assert(x)
> >> > +#else
> >> > +#define lldbassert(x) lldb_private::lldb_assert(x, #x, __FUNCTION__,
> __FILE__, __LINE__)
> >> > +#endif
> >>
> >> Why should we have this ifdef? As in, why shouldn't we use lldb_assert
> >> unconditionally, always (giving us the benefit of backtraces always)?
> >> _______________________________________________
> >> lldb-commits mailing list
> >> lldb-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> > Thanks,
> > - Enrico
> > 📩 egranata@.com ☎️ 27683
> >
> >
> >
> >
> > _______________________________________________
> > 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/59512a97/attachment.html>