[Lldb-commits] [PATCH] D23026: [LLDB-MI] removing requirement of a parameter for -break-insert's -f flag

[Ilia K via lldb-commits]

ki.stfu requested changes to this revision.
ki.stfu added a comment.
This revision now requires changes to proceed.

Hi @pieandcakes!

I'm sorry for the delay, have a lot of work. Please folllow my inline comments, rebase against ToT and update CL's summary.


================
Comment at: tools/lldb-mi/MICmdCmdBreak.cpp:156-163
@@ -156,8 +155,10 @@
     m_bBrkPtIsPending = pArgPendingBrkPt->GetFound();
     if (pArgLocation->GetFound())
         m_brkName = pArgLocation->GetValue();
-    else if (m_bBrkPtIsPending)
+    else 
     {
-        pArgPendingBrkPt->GetExpectedOption<CMICmdArgValString, CMIUtilString>(m_brkName);
+        SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_BRKPT_BAD_LOCATION), m_cmdData.strMiCmd.c_str()));
+        return MIstatus::failure;
     }
+
     if (pArgIgnoreCnt->GetFound())
----------------
I think we can remove these lines and make pArgLocation mandatory.
For this, please specify it during registration of m_constStrArgNamedLocation in CMICmdCmdBreakInsert::ParseArgs, and then replaces the highlighted lines with:
```
m_brkName = pArgLocation->GetValue();
```

================
Comment at: tools/lldb-mi/MICmnResources.cpp:222
@@ -221,2 +221,3 @@
     {IDS_CMD_ERR_BRKPT_CNT_EXCEEDED, "Command '%s'. Number of valid breakpoint exceeded %d. Cannot create new breakpoint '%s'"},
+    {IDS_CMD_ERR_BRKPT_BAD_LOCATION, "Command '%s'. Unable to parse breakpoint location."},
     {IDS_CMD_ERR_SOME_ERROR, "Command '%s'. Error: %s"},
----------------
revert this

================
Comment at: tools/lldb-mi/MICmnResources.h:239
@@ -238,2 +238,3 @@
     IDS_CMD_ERR_BRKPT_CNT_EXCEEDED,
+    IDS_CMD_ERR_BRKPT_BAD_LOCATION,
     IDS_CMD_ERR_SOME_ERROR,
----------------
revert this


Repository:
  rL LLVM

https://reviews.llvm.org/D23026