[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

This revision was automatically updated to reflect the committed changes. Closed by commit rL357268: Don't abort() in lldb_assert and document why. (authored by adrian, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://review

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl added a comment. Another problem with `lldb_assert()` is that in its current form it prints to stderr, which AFAIK is not even visible when LLDB is invoked from an IDE like Xcode. I like Pavel's suggestion of having a report_once function. It looks like everybody seems to agree that ll

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

labath accepted this revision. labath added a comment. In D59911#1446942 , @jingham wrote: > In this case it might be better to print an error at the problem site. You > could certainly do this with an assert and a printf. lldbassert just > provides a

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

jingham added a comment. In D59911#1446946 , @zturner wrote: > In D59911#1446942 , @jingham wrote: > > > In D59911#1446745 , @davide wrote: > > > > > My (maybe unpopolar) opi

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

zturner added a comment. In D59911#1446942 , @jingham wrote: > In D59911#1446745 , @davide wrote: > > > My (maybe unpopolar) opinion on the subject is that "soft assertions" are a > > way to cleanse your conscience

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

jingham added a comment. In D59911#1446745 , @davide wrote: > My (maybe unpopolar) opinion on the subject is that "soft assertions" are a > way to cleanse your conscience of guilt, but they don't work really well in > practice. > When I started working

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Works for me. In D59911#1445917 , @labath wrote: > In D59911#1445392 , @JDevlieghere > wrote: > >

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl added a comment. In D59911#1446835 , @davide wrote: > > I do personally believe that your wording is reasonable. If it was me, I > would just be a little stronger and say that new code should never use > lldbassert, and if you're touching exi

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

davide accepted this revision. davide added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/D59911 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl updated this revision to Diff 192730. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/D59911 Files: lldb/docs/index.rst lldb/docs/resources/source.rst lldb/source/Utility/LLDBAssert.cpp lldb/www/source.html Index: lldb/www/source.html ==

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

xiaobai added a comment. Thank you for taking the time to write this up! I wish I could have read this when I started working on LLDB. :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/D59911 ___ lldb-com

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

davide added a comment. In D59911#1446787 , @aprantl wrote: > Thanks for summarizing your thoughts, Davide. > > I think that what you wrote is a much better explanation of what I was trying > to say with > > Use these sparingly and only if error handlin

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl added a comment. Thanks for summarizing your thoughts, Davide. I think that what you wrote is a much better explanation of what I was trying to say with Use these sparingly and only if error handling is not otherwise feasible. I think that unless we can eliminate all uses of lldb_ass

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

davide added a comment. My (maybe unpopolar) opinion on the subject is that "soft assertions" are a way to cleanse your conscience of guilt, but they don't work really well in practice. When I started working on lldb, I was a fairly strong proponent of assertions everywhere. My view changed som

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl added a comment. While writing this I came to the conclusion that `lldb_assert()` is really a lazy way of handling errors. If we can we should replace it with error handling and perhaps logging. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl updated this revision to Diff 192708. aprantl added a comment. Address review feedback! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/D59911 Files: lldb/docs/index.rst lldb/docs/resources/source.rst lldb/source/Utility/LLDBAssert.cpp

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

labath added a comment. Thank you for writing this up. I think there is a lot of confusion around this topic, and a document like this is a good step towards clearing it up. Not really related to this patch, but since we're talking about lldb_assert... The thing that has always bugged me about

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl updated this revision to Diff 192548. Herald added a subscriber: arphaman. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/D59911 Files: lldb/docs/index.rst lldb/docs/resources/source.rst lldb/source/Utility/LLDBAssert.cpp lldb/www/source

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl marked an inline comment as done. aprantl added inline comments. Comment at: lldb/www/source.html:83 + Assertions. +Assertions (from assert.h) should be used liberally to assert internal consistency. +

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

JDevlieghere requested changes to this revision. JDevlieghere added a comment. This revision now requires changes to proceed. Can you please also update the RST docs? (https://lldb.llvm.org/new-www/) Comment at: lldb/www/source.html:83 + Assertions. +

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

aprantl created this revision. aprantl added reviewers: jingham, clayborg, labath, zturner, jasonmolenda. Herald added a project: LLDB. See the changes to the website for the full rationale. Having a policy document that explains when to use what method of error handling in LLDB will make on-boa