teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

In D64267#1571962 <https://reviews.llvm.org/D64267#1571962>, @JDevlieghere 
wrote:

> In D64267#1571960 <https://reviews.llvm.org/D64267#1571960>, @teemperor wrote:
>
> > Would it be possible to assert after the printing the stack trace? Having 
> > stack traces in bug reports is always nice to have and displaying the 
> > "Assertion failed: ..." message seems also useful for the user.
>
>
> The stack trace will be printed when the `assert` is hit, because we have a 
> call to LLVM's pretty stack trace printer in the signal handler. That's the 
> reason I moved the assert *before* the `PrintStackTrace` call.
>
> Regarding the `Assertion failed` message, I wasn't sure if we wanted to print 
> that twice, once coming form `lldb_assert` (with the real function, file and 
> line) and once from the `assert` (with the function, file and line pointing 
> to the lldb_assert implementation). If you think it's useful I'm happy to 
> move the assertion to just after that print statement.


Ah, thanks for the explanation! The message was more of a positive side-effect 
of moving it afterwards, but I don't think it's worth moving this just for that 
message. So I think the current version is good to go then.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64267/new/

https://reviews.llvm.org/D64267



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to