Page MenuHomePhabricator

Handle the options and parameters separator in every MI command
ClosedPublic

Authored by abidh on Oct 30 2015, 4:33 AM.

Details

Summary

As per the following link, the "--" separator can appear between the options
and parameters of any MI command. Previously this separator was only
handled by the -data-disassemble MI command. I have moved the relevant
code into CMICmdBase so that any MI command can handle the
aforementioned separator.

https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Input-Syntax.html#GDB_002fMI-Input-Syntax

Diff Detail

Event Timeline

abidh updated this revision to Diff 38792.Oct 30 2015, 4:33 AM
abidh retitled this revision from to Handle the option and parameter separator in every command..
abidh updated this object.
abidh added a reviewer: ki.stfu.
abidh added a subscriber: lldb-commits.
enlight retitled this revision from Handle the option and parameter separator in every command. to Handle the options and parameters separator in every MI command.Oct 30 2015, 6:59 AM
enlight updated this object.
ki.stfu requested changes to this revision.Nov 1 2015, 11:11 PM
ki.stfu edited edge metadata.
ki.stfu added inline comments.
tools/lldb-mi/MICmdBase.cpp
102

The -data-disassemble command has '--' mandatory argument, but now it's always optional argument. Should it use m_ConsumeArgMandatory as it works for --thread/--frame etc?

tools/lldb-mi/MICmdCmdData.cpp
280

Please expand MiDataTestCase.test_lldbmi_data_disassemble test to check that it still gives an error without '--' argument.

This revision now requires changes to proceed.Nov 1 2015, 11:11 PM
abidh updated this revision to Diff 38879.Nov 2 2015, 2:06 AM
abidh edited edge metadata.

Handled review comment.

Added a test for -data-disassemble without "--" separator.

abidh added inline comments.Nov 2 2015, 2:07 AM
tools/lldb-mi/MICmdBase.cpp
102

I changed this intentionally because "--" should not be mandatory. The MI specifications does not make it mandatory and in GDB, disassemble command works with and without it. So this change bring the same behavior in lldb-mi.

tools/lldb-mi/MICmdCmdData.cpp
280

I have added a test that check the -data-disassemble without "--" as it is not mandatory.

ki.stfu accepted this revision.Nov 2 2015, 3:02 AM
ki.stfu edited edge metadata.

lgtm

tools/lldb-mi/MICmdBase.cpp
102

Well, I thought it's mandatory argument according to its spec: https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Data-Manipulation.html

-data-disassemble
   [ -s start-addr -e end-addr ]
 | [ -f filename -l linenum [ -n lines ] ]
 -- mode

But I think we can follow to the GDB syntax.

tools/lldb-mi/MICmdCmdData.cpp
280

ok, thanks.

This revision is now accepted and ready to land.Nov 2 2015, 3:02 AM
abidh marked an inline comment as done.Nov 2 2015, 3:37 AM
abidh added inline comments.
tools/lldb-mi/MICmdBase.cpp
102

If you read the following, you will see that "--" is not mandatory. It says "Options occur first in the parameter list and can be delimited from normal parameters using ‘--’ (this is useful when some parameters begin with a dash)."

https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Input-Syntax.html#GDB_002fMI-Input-Syntax

abidh closed this revision.Nov 2 2015, 3:45 AM
abidh marked an inline comment as done.
ki.stfu added inline comments.Nov 2 2015, 5:23 AM
tools/lldb-mi/MICmdBase.cpp
102

As for me, it says that dashes aren't mandatory in general cases (i.e. unless otherwise specified in command's spec). In case of -data-disassemble, we know that mode is either 0, 1, 2, or 3, and therefore according to your link, the syntax would be:

-data-disassemble [ -s start-addr -e end-addr ] | [ -f filename -l linenum [ -n lines ] ] [--] mode

(i.e. "--" is optional)

But the spec says that '--' is mandatory argument and I perceive it as a special case of usage '--'. Therefore I supposed that it's mandatory argument.