teemperor added a comment. In D104778#2835701 <https://reviews.llvm.org/D104778#2835701>, @DavidSpickett wrote:
>> LGTM, I'll add them in my patch. Thanks for the quick turnaround! > > To clarify, is there still a regression with the API? No regression anymore from what I can see. My point was that I'll fix the usage from the SB API and then I can just re-add the asserts in the same patch (the asserts themselves are a good idea, the SB API just should have been updated to prevent hitting them from user scripts) > - This removes the asserts so you won't crash > - No functions are changed in `lldb/include/lldb/API/SBCommandReturnObject.h` > > The one thing that is different is that calling the API SetError with an > empty string will now set eReturnStatusFailed. So: > > s = do_thing_and_return_err_string_if_err(...) > return_object.SetError(s) > > Would now set it to failed even if s is empty. > > Perhaps the SetError(...) functions should retain the empty message == nop > behaviour and AppendError can set the status unconditionally? I think the assert behaviour is fine for the internal API, but the SB API behaviour is a bit more tricky. But let's discuss the expected behaviour in a dedicated review for that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104778/new/ https://reviews.llvm.org/D104778 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
