[Lldb-commits] [PATCH] D24202: [lldb-mi] Fix parsing expressions to evaluate with spaces and optional args

[Ed Munoz via lldb-commits]

edmunoz created this revision.
edmunoz added a reviewer: ki.stfu.
edmunoz added a subscriber: lldb-commits.
edmunoz set the repository for this revision to rL LLVM.
edmunoz added a project: LLDB.
Herald added a subscriber: ki.stfu.

When extracting options for long options (starting with "--"), the use of
MIUtilString::SplitConsiderQuotes to split all the arguments was being
conditioned on the option type to be expected. This was wrong as this caused
other options to be parsed incorrectly since it was not taking into account the
presence of quotes.

Repository:
  rL LLVM

https://reviews.llvm.org/D24202

Files:
  packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
  tools/lldb-mi/MICmdArgValOptionLong.cpp

Index: tools/lldb-mi/MICmdArgValOptionLong.cpp
===
--- tools/lldb-mi/MICmdArgValOptionLong.cpp
+++ tools/lldb-mi/MICmdArgValOptionLong.cpp
@@ -187,15 +187,9 @@
 bool
 CMICmdArgValOptionLong::ExtractExpectedOptions(CMICmdArgContext &vrwTxt, const 
MIuint nArgIndex)
 {
-CMIUtilString::VecString_t vecOptions;
-MIuint nOptionsPresent = 0;
-if ((m_eExpectingOptionType != eArgValType_StringQuoted) && 
(m_eExpectingOptionType != eArgValType_StringQuotedNumber) &&
-(m_eExpectingOptionType != eArgValType_StringQuotedNumberPath))
-nOptionsPresent = vrwTxt.GetArgsLeftToParse().Split(" ", vecOptions);
-else
-nOptionsPresent = vrwTxt.GetArgsLeftToParse().SplitConsiderQuotes(" ", 
vecOptions);
-if (nOptionsPresent == 0)
+if (vrwTxt.GetNumberArgsPresent() == 0)
 return MIstatus::failure;
+CMIUtilString::VecString_t vecOptions = vrwTxt.GetArgs();
 
 MIuint nArgIndexCnt = 0;
 MIuint nTypeCnt = 0;
Index: packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
===
--- packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
+++ packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
@@ -128,6 +128,12 @@
 # FIXME: The name below is not correct. It should be "var.*argv[0]".
 
self.expect("\^done,numchild=\"1\",children=\[child=\{name=\"var6\.\*\$[0-9]+\",exp=\"\*\$[0-9]+\",numchild=\"0\",type=\"const
 char\",thread-id=\"4294967295\",value=\"47 
'/'\",has_more=\"0\"\}\],has_more=\"0\"") #FIXME -var-list-children shows 
invalid thread-id
 
+# Print an expression with spaces and optional arguments
+self.runCmd("-data-evaluate-expression \"a + b\"")
+self.expect("\^done,value=\"12\"")
+self.runCmd("-var-create var7 * \"a + b\" --thread 1 --frame 0")
+
self.expect("\^done,name=\"var7\",numchild=\"0\",value=\"12\",type=\"int\",thread-id=\"1\",has_more=\"0\"")
+
 @skipIfWindows #llvm.org/pr24452: Get lldb-mi tests working on Windows
 @skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread 
races
 @skipIfLinux # llvm.org/pr22841: lldb-mi tests fail on all Linux buildbots


Index: tools/lldb-mi/MICmdArgValOptionLong.cpp
===
--- tools/lldb-mi/MICmdArgValOptionLong.cpp
+++ tools/lldb-mi/MICmdArgValOptionLong.cpp
@@ -187,15 +187,9 @@
 bool
 CMICmdArgValOptionLong::ExtractExpectedOptions(CMICmdArgContext &vrwTxt, const MIuint nArgIndex)
 {
-CMIUtilString::VecString_t vecOptions;
-MIuint nOptionsPresent = 0;
-if ((m_eExpectingOptionType != eArgValType_StringQuoted) && (m_eExpectingOptionType != eArgValType_StringQuotedNumber) &&
-(m_eExpectingOptionType != eArgValType_StringQuotedNumberPath))
-nOptionsPresent = vrwTxt.GetArgsLeftToParse().Split(" ", vecOptions);
-else
-nOptionsPresent = vrwTxt.GetArgsLeftToParse().SplitConsiderQuotes(" ", vecOptions);
-if (nOptionsPresent == 0)
+if (vrwTxt.GetNumberArgsPresent() == 0)
 return MIstatus::failure;
+CMIUtilString::VecString_t vecOptions = vrwTxt.GetArgs();
 
 MIuint nArgIndexCnt = 0;
 MIuint nTypeCnt = 0;
Index: packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
===
--- packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
+++ packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
@@ -128,6 +128,12 @@
 # FIXME: The name below is not correct. It should be "var.*argv[0]".
 self.expect("\^done,numchild=\"1\",children=\[child=\{name=\"var6\.\*\$[0-9]+\",exp=\"\*\$[0-9]+\",numchild=\"0\",type=\"const char\",thread-id=\"4294967295\",value=\"47 '/'\",has_more=\"0\"\}\],has_more=\"0\"") #FIXME -var-list-children shows invalid thread-id
 
+# Print an expression with spaces and optional arguments
+self.runCmd("-data-evaluate-expression \"a + b\"")
+self.expect("\^done,value=\"12\"")
+self.runCmd("-var-create var7 * \"a + b\" --thread 1 --frame 0")
+self.expect("\^done,name=\"var7\",numchild=\

Re: [Lldb-commits] [PATCH] D24202: [lldb-mi] Fix parsing expressions to evaluate with spaces and optional args

[Ed Munoz via lldb-commits]

edmunoz added inline comments.


Comment at: tools/lldb-mi/MICmdArgValOptionLong.cpp:190
@@ -189,10 +189,3 @@
 {
-CMIUtilString::VecString_t vecOptions;
-MIuint nOptionsPresent = 0;
-if ((m_eExpectingOptionType != eArgValType_StringQuoted) && 
(m_eExpectingOptionType != eArgValType_StringQuotedNumber) &&
-(m_eExpectingOptionType != eArgValType_StringQuotedNumberPath))
-nOptionsPresent = vrwTxt.GetArgsLeftToParse().Split(" ", vecOptions);
-else
-nOptionsPresent = vrwTxt.GetArgsLeftToParse().SplitConsiderQuotes(" ", 
vecOptions);
-if (nOptionsPresent == 0)
+if (vrwTxt.GetNumberArgsPresent() == 0)
 return MIstatus::failure;

We could use the size of `vrwTxt.GetArgs()` to reduce the number of calls to 
`CMIUtilString::SplitConsiderQuotes` in `CMICmdArgContext`.

I left it using the more expressive method name, but I'm open to changing it.


Repository:
  rL LLVM

https://reviews.llvm.org/D24202



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D24202: [lldb-mi] Fix parsing expressions to evaluate with spaces and optional args

[Ed Munoz via lldb-commits]

edmunoz added a comment.

In https://reviews.llvm.org/D24202#539629, @ki.stfu wrote:

> lgtm
>
> @edmunoz, good job. Thank you! Would you like me to commit?


Thanks! Yes, go ahead.


Repository:
  rL LLVM

https://reviews.llvm.org/D24202



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits