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 <egran...@apple.com> wrote: > On Mar 4, 2015, at 5:18 PM, Zachary Turner <ztur...@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.enr...@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@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits > > >
_______________________________________________ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits