[Lldb-commits] [PATCH] D32366: Set "success" status correctly

jafl added a comment. Absolutely! https://reviews.llvm.org/D32366 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D32366: Set "success" status correctly

clayborg added a comment. This is sure to trigger things in the test suite. We will need to ensure a few things: - test suite runs cleanly in debug mode after the lldbassert is added - without changes to CommandObjectRegister.cpp that the lldbassert is triggered, and if not, add a test https:

[Lldb-commits] [PATCH] D32366: Set "success" status correctly

jafl updated this revision to Diff 112922. jafl added a comment. Added lldbassert https://reviews.llvm.org/D32366 Files: source/Commands/CommandObjectRegister.cpp source/Interpreter/CommandInterpreter.cpp Index: source/Interpreter/CommandInterpreter.cpp ===

[Lldb-commits] [PATCH] D32366: Set "success" status correctly

jafl added a comment. I will try to ensure that the lldbassert gets tested, but right now, none of the tests work on my machine. I emailed lldb-dev for help. Repository: rL LLVM https://reviews.llvm.org/D32366 ___ lldb-commits mailing list lldb

[Lldb-commits] [PATCH] D32366: Set "success" status correctly

clayborg added a comment. Yes: lldbassert would be fine for that since those get compiled out during release. Patch looks fine. If we already have a test that would trigger the new "lldbassert" you will add, then no need for a special test for this, else we need a test that triggers this. Rep

[Lldb-commits] [PATCH] D32366: Set "success" status correctly

jafl added a comment. Thanks for the suggestions. I will get around to this - just swamped right now! Repository: rL LLVM https://reviews.llvm.org/D32366 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[Lldb-commits] [PATCH] D32366: Set "success" status correctly

labath added a comment. that sounds like an excellent idea, as it will check all executed commands, and not the ones we've remembered checking. It should probably be an `lldbassert` though. (And we'd need to check that the existing tests still pass after that.) Repository: rL LLVM https://r

[Lldb-commits] [PATCH] D32366: Set "success" status correctly

tatyana-krasnukha added a comment. It can make sense to add static_assert(eReturnStatusStarted != result); after cmd_obj->Execute() invocation at CommandInterpreter::HandleCommand, do you think? Repository: rL LLVM https://reviews.llvm.org/D32366 __

[Lldb-commits] [PATCH] D32366: Set "success" status correctly

labath added a comment. Thanks for the patch. Could you please also add an appropriate test for it? Doing something similar to what `packages/Python/lldbsuite/test/functionalities/frame_var/TestFrameVar.py` does should be the easiest way to test this. Repository: rL LLVM https://reviews.ll

[Lldb-commits] [PATCH] D32366: Set "success" status correctly

jafl created this revision. jafl added a project: LLDB. In CommandObjectRegisterRead.DoExecute(), set the status to "success" when appropriate. Repository: rL LLVM https://reviews.llvm.org/D32366 Files: source/Commands/CommandObjectRegister.cpp Index: source/Commands/CommandObjectRegist