[Lldb-commits] [PATCH] D24711: [lldb-mi] Fix implementation for a few mi commands

[Aetf via Phabricator via lldb-commits]

aetf added a comment.

Hi, sorry for the long delay. It has been a busy semester.

Added unit tests are

- `MiGdbSetShowTestCase.test_lldbmi_gdb_set_disassembly_flavor` for disassembly flavor settings. Note this one doesn't pass currently due to https://llvm.org/bugs/show_bug.cgi?id=31485
- `MiEnvironmentCdTestCase.test_lldbmi_environment_cd` for `-environment-cd` command

Extended test case:

- `MiSyntaxTestCase.test_lldbmi_output_grammar` to include a command generating console stream records

Fixed test case:

- `MiExecTestCase.test_lldbmi_exec_arguments_set` which breaks because of the console stream records

I tested against latest trunk version (290647), and all tests passed with 5 expected failures and 3 unexpected success:

- `MiGdbSetShowTestCase.test_lldbmi_gdb_set_target_async_off`
- `MiInterpreterExecTestCase.test_lldbmi_settings_set_target_run_args_after`
- `MiSyntaxTestCase.test_lldbmi_process_output`

I don't have the commit access, but I can create separate RRs if necessary. (should the target select error fix go to it's own RR?)



================
Comment at: tools/lldb-mi/MICmdCmdMiscellanous.cpp:515
+      return MIstatus::failure;
   }
 
----------------
abidh wrote:
> It is not really an OutofBand record but rather an output of the command. Why not simple add prepend an ~
The output of the command should be a Stream record which is an OutofBand record according to the spec [1]. I agree it's no more than prepending '~' and quoting the string. But why not just do what the spec says? ;-)

[1] https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Stream-Records.html#GDB_002fMI-Stream-Records


================
Comment at: tools/lldb-mi/MICmdCmdTarget.cpp:126
   lldb::SBStream errMsg;
+  error.GetDescription(errMsg);
   if (!process.IsValid()) {
----------------
abidh wrote:
> This does not seem related to any bug fix.
I didn't create a bug report for this since it's not a big deal. But it's rather annoying when debugging because the actual error description never gets to the output.


https://reviews.llvm.org/D24711