[Lldb-commits] [lldb] r373673 - Break out the Python class & key/value options into a separate OptionGroup.
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Thu Oct 3 15:18:51 PDT 2019
Author: jingham
Date: Thu Oct 3 15:18:51 2019
New Revision: 373673
URL: http://llvm.org/viewvc/llvm-project?rev=373673&view=rev
Log:
Break out the Python class & key/value options into a separate OptionGroup.
Use this in the scripted breakpoint command. Added some tests for parsing
the key/value options. This uncovered a bug in handling parsing errors mid-line.
I also fixed that bug.
Differential Revision: https://reviews.llvm.org/D68363
Added:
lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp
Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/resolver.py
lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp
lldb/trunk/source/Commands/Options.td
lldb/trunk/source/Interpreter/CMakeLists.txt
lldb/trunk/source/Interpreter/Options.cpp
Added: lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h?rev=373673&view=auto
==============================================================================
--- lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h (added)
+++ lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h Thu Oct 3 15:18:51 2019
@@ -0,0 +1,64 @@
+//===-- OptionGroupPythonClassWithDict.h -------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef liblldb_OptionGroupString_h_
+#define liblldb_OptionGroupString_h_
+
+#include "lldb/Interpreter/Options.h"
+#include "lldb/Utility/StructuredData.h"
+
+namespace lldb_private {
+
+// Use this Option group if you have a python class that implements some
+// Python extension point, and you pass a SBStructuredData to the class
+// __init__ method.
+// class_option specifies the class name
+// the key and value options are read in in pairs, and a
+// StructuredData::Dictionary is constructed with those pairs.
+class OptionGroupPythonClassWithDict : public OptionGroup {
+public:
+ OptionGroupPythonClassWithDict(const char *class_use,
+ int class_option = 'C',
+ int key_option = 'k',
+ int value_option = 'v',
+ char *class_long_option = "python-class",
+ const char *key_long_option = "python-class-key",
+ const char *value_long_option = "python-class-value",
+ bool required = false);
+
+ ~OptionGroupPythonClassWithDict() override;
+
+ llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
+ return llvm::ArrayRef<OptionDefinition>(m_option_definition);
+ }
+
+ Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value,
+ ExecutionContext *execution_context) override;
+ Status SetOptionValue(uint32_t, const char *, ExecutionContext *) = delete;
+
+ void OptionParsingStarting(ExecutionContext *execution_context) override;
+ Status OptionParsingFinished(ExecutionContext *execution_context) override;
+
+ const StructuredData::DictionarySP GetStructuredData() {
+ return m_dict_sp;
+ }
+ const std::string &GetClassName() {
+ return m_class_name;
+ }
+
+protected:
+ std::string m_class_name;
+ std::string m_current_key;
+ StructuredData::DictionarySP m_dict_sp;
+ std::string m_class_usage_text, m_key_usage_text, m_value_usage_text;
+ OptionDefinition m_option_definition[3];
+};
+
+} // namespace lldb_private
+
+#endif // liblldb_OptionGroupString_h_
Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py?rev=373673&r1=373672&r2=373673&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py Thu Oct 3 15:18:51 2019
@@ -38,6 +38,12 @@ class TestScriptedResolver(TestBase):
self.build()
self.do_test_cli()
+ def test_bad_command_lines(self):
+ """Make sure we get appropriate errors when we give invalid key/value
+ options"""
+ self.build()
+ self.do_test_bad_options()
+
def setUp(self):
# Call super's setUp().
TestBase.setUp(self)
@@ -195,3 +201,15 @@ class TestScriptedResolver(TestBase):
target = self.make_target_and_import()
lldbutil.run_break_set_by_script(self, "resolver.Resolver", extra_options="-k symbol -v break_on_me")
+
+ def do_test_bad_options(self):
+ target = self.make_target_and_import()
+
+ self.expect("break set -P resolver.Resolver -k a_key", error = True, msg="Missing value at end",
+ substrs=['Key: "a_key" missing value'])
+ self.expect("break set -P resolver.Resolver -v a_value", error = True, msg="Missing key at end",
+ substrs=['Value: "a_value" missing matching key'])
+ self.expect("break set -P resolver.Resolver -v a_value -k a_key -v another_value", error = True, msg="Missing key among args",
+ substrs=['Value: "a_value" missing matching key'])
+ self.expect("break set -P resolver.Resolver -k a_key -k a_key -v another_value", error = True, msg="Missing value among args",
+ substrs=['Key: "a_key" missing value'])
Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/resolver.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/resolver.py?rev=373673&r1=373672&r2=373673&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/resolver.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/resolver.py Thu Oct 3 15:18:51 2019
@@ -7,6 +7,7 @@ class Resolver:
def __init__(self, bkpt, extra_args, dict):
self.bkpt = bkpt
self.extra_args = extra_args
+
Resolver.func_list = []
Resolver.got_files = 0
@@ -15,6 +16,8 @@ class Resolver:
sym_item = self.extra_args.GetValueForKey("symbol")
if sym_item.IsValid():
sym_name = sym_item.GetStringValue(1000)
+ else:
+ print("Didn't have a value for key 'symbol'")
if sym_ctx.compile_unit.IsValid():
Resolver.got_files = 1
Modified: lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp?rev=373673&r1=373672&r2=373673&view=diff
==============================================================================
--- lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp Thu Oct 3 15:18:51 2019
@@ -16,6 +16,7 @@
#include "lldb/Interpreter/CommandInterpreter.h"
#include "lldb/Interpreter/CommandReturnObject.h"
#include "lldb/Interpreter/OptionArgParser.h"
+#include "lldb/Interpreter/OptionGroupPythonClassWithDict.h"
#include "lldb/Interpreter/OptionValueBoolean.h"
#include "lldb/Interpreter/OptionValueString.h"
#include "lldb/Interpreter/OptionValueUInt64.h"
@@ -241,12 +242,16 @@ public:
interpreter, "breakpoint set",
"Sets a breakpoint or set of breakpoints in the executable.",
"breakpoint set <cmd-options>"),
+ m_python_class_options("scripted breakpoint", 'P'),
m_bp_opts(), m_options() {
// We're picking up all the normal options, commands and disable.
+ m_all_options.Append(&m_python_class_options, LLDB_OPT_SET_1,
+ LLDB_OPT_SET_11);
m_all_options.Append(&m_bp_opts,
LLDB_OPT_SET_1 | LLDB_OPT_SET_3 | LLDB_OPT_SET_4,
LLDB_OPT_SET_ALL);
- m_all_options.Append(&m_dummy_options, LLDB_OPT_SET_1, LLDB_OPT_SET_ALL);
+ m_all_options.Append(&m_dummy_options, LLDB_OPT_SET_1,
+ LLDB_OPT_SET_ALL);
m_all_options.Append(&m_options);
m_all_options.Finalize();
}
@@ -352,14 +357,6 @@ public:
m_hardware = true;
break;
- case 'k': {
- if (m_current_key.empty())
- m_current_key.assign(option_arg);
- else
- error.SetErrorStringWithFormat("Key: %s missing value.",
- m_current_key.c_str());
-
- } break;
case 'K': {
bool success;
bool value;
@@ -441,10 +438,6 @@ public:
m_source_text_regexp.assign(option_arg);
break;
- case 'P':
- m_python_class.assign(option_arg);
- break;
-
case 'r':
m_func_regexp.assign(option_arg);
break;
@@ -458,16 +451,6 @@ public:
m_func_name_type_mask |= eFunctionNameTypeSelector;
break;
- case 'v': {
- if (!m_current_key.empty()) {
- m_extra_args_sp->AddStringItem(m_current_key, option_arg);
- m_current_key.clear();
- }
- else
- error.SetErrorStringWithFormat("Value \"%s\" missing matching key.",
- option_arg.str().c_str());
- } break;
-
case 'w': {
bool success;
m_throw_bp = OptionArgParser::ToBoolean(option_arg, true, &success);
@@ -510,8 +493,6 @@ public:
m_exception_extra_args.Clear();
m_move_to_nearest_code = eLazyBoolCalculate;
m_source_regex_func_names.clear();
- m_python_class.clear();
- m_extra_args_sp = std::make_shared<StructuredData::Dictionary>();
m_current_key.clear();
}
@@ -543,8 +524,6 @@ public:
Args m_exception_extra_args;
LazyBool m_move_to_nearest_code;
std::unordered_set<std::string> m_source_regex_func_names;
- std::string m_python_class;
- StructuredData::DictionarySP m_extra_args_sp;
std::string m_current_key;
};
@@ -565,7 +544,7 @@ protected:
BreakpointSetType break_type = eSetTypeInvalid;
- if (!m_options.m_python_class.empty())
+ if (!m_python_class_options.GetClassName().empty())
break_type = eSetTypeScripted;
else if (m_options.m_line_num != 0)
break_type = eSetTypeFileAndLine;
@@ -721,9 +700,9 @@ protected:
Status error;
bp_sp = target.CreateScriptedBreakpoint(
- m_options.m_python_class, &(m_options.m_modules),
+ m_python_class_options.GetClassName().c_str(), &(m_options.m_modules),
&(m_options.m_filenames), false, m_options.m_hardware,
- m_options.m_extra_args_sp, &error);
+ m_python_class_options.GetStructuredData(), &error);
if (error.Fail()) {
result.AppendErrorWithFormat(
"Error setting extra exception arguments: %s",
@@ -818,6 +797,7 @@ private:
BreakpointOptionGroup m_bp_opts;
BreakpointDummyOptionGroup m_dummy_options;
+ OptionGroupPythonClassWithDict m_python_class_options;
CommandOptions m_options;
OptionGroupOptions m_all_options;
};
Modified: lldb/trunk/source/Commands/Options.td
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/Options.td?rev=373673&r1=373672&r2=373673&view=diff
==============================================================================
--- lldb/trunk/source/Commands/Options.td (original)
+++ lldb/trunk/source/Commands/Options.td Thu Oct 3 15:18:51 2019
@@ -163,17 +163,6 @@ let Command = "breakpoint set" in {
"match against all source files, pass the \"--all-files\" option.">;
def breakpoint_set_all_files : Option<"all-files", "A">, Group<9>,
Desc<"All files are searched for source pattern matches.">;
- def breakpoint_set_python_class : Option<"python-class", "P">, Group<11>,
- Arg<"PythonClass">, Required, Desc<"The name of the class that implement "
- "a scripted breakpoint.">;
- def breakpoint_set_python_class_key : Option<"python-class-key", "k">,
- Group<11>, Arg<"None">,
- Desc<"The key for a key/value pair passed to the class that implements a "
- "scripted breakpoint. Can be specified more than once.">;
- def breakpoint_set_python_class_value : Option<"python-class-value", "v">,
- Group<11>, Arg<"None">, Desc<"The value for the previous key in the pair "
- "passed to the class that implements a scripted breakpoint. "
- "Can be specified more than once.">;
def breakpoint_set_language_exception : Option<"language-exception", "E">,
Group<10>, Arg<"Language">, Required,
Desc<"Set the breakpoint on exceptions thrown by the specified language "
Modified: lldb/trunk/source/Interpreter/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/CMakeLists.txt?rev=373673&r1=373672&r2=373673&view=diff
==============================================================================
--- lldb/trunk/source/Interpreter/CMakeLists.txt (original)
+++ lldb/trunk/source/Interpreter/CMakeLists.txt Thu Oct 3 15:18:51 2019
@@ -20,6 +20,7 @@ add_lldb_library(lldbInterpreter
OptionGroupBoolean.cpp
OptionGroupFile.cpp
OptionGroupFormat.cpp
+ OptionGroupPythonClassWithDict.cpp
OptionGroupOutputFile.cpp
OptionGroupPlatform.cpp
OptionGroupString.cpp
Added: lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp?rev=373673&view=auto
==============================================================================
--- lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp (added)
+++ lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp Thu Oct 3 15:18:51 2019
@@ -0,0 +1,124 @@
+//===-- OptionGroupKeyValue.cpp ----------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Interpreter/OptionGroupPythonClassWithDict.h"
+
+#include "lldb/Host/OptionParser.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+OptionGroupPythonClassWithDict::OptionGroupPythonClassWithDict
+ (const char *class_use,
+ int class_option,
+ int key_option,
+ int value_option,
+ char *class_long_option,
+ const char *key_long_option,
+ const char *value_long_option,
+ bool required) {
+ m_key_usage_text.assign("The key for a key/value pair passed to the class"
+ " that implements a ");
+ m_key_usage_text.append(class_use);
+ m_key_usage_text.append(". Pairs can be specified more than once.");
+
+ m_value_usage_text.assign("The value for a previous key in the pair passed to"
+ " the class that implements a ");
+ m_value_usage_text.append(class_use);
+ m_value_usage_text.append(". Pairs can be specified more than once.");
+
+ m_class_usage_text.assign("The name of the class that will manage a ");
+ m_class_usage_text.append(class_use);
+ m_class_usage_text.append(".");
+
+ m_option_definition[0].usage_mask = LLDB_OPT_SET_1;
+ m_option_definition[0].required = required;
+ m_option_definition[0].long_option = class_long_option;
+ m_option_definition[0].short_option = class_option;
+ m_option_definition[0].validator = nullptr;
+ m_option_definition[0].option_has_arg = OptionParser::eRequiredArgument;
+ m_option_definition[0].enum_values = {};
+ m_option_definition[0].completion_type = 0;
+ m_option_definition[0].argument_type = eArgTypePythonClass;
+ m_option_definition[0].usage_text = m_class_usage_text.data();
+
+ m_option_definition[1].usage_mask = LLDB_OPT_SET_1;
+ m_option_definition[1].required = required;
+ m_option_definition[1].long_option = key_long_option;
+ m_option_definition[1].short_option = key_option;
+ m_option_definition[1].validator = nullptr;
+ m_option_definition[1].option_has_arg = OptionParser::eRequiredArgument;
+ m_option_definition[1].enum_values = {};
+ m_option_definition[1].completion_type = 0;
+ m_option_definition[1].argument_type = eArgTypeNone;
+ m_option_definition[1].usage_text = m_key_usage_text.data();
+
+ m_option_definition[2].usage_mask = LLDB_OPT_SET_1;
+ m_option_definition[2].required = required;
+ m_option_definition[2].long_option = value_long_option;
+ m_option_definition[2].short_option = value_option;
+ m_option_definition[2].validator = nullptr;
+ m_option_definition[2].option_has_arg = OptionParser::eRequiredArgument;
+ m_option_definition[2].enum_values = {};
+ m_option_definition[2].completion_type = 0;
+ m_option_definition[2].argument_type = eArgTypeNone;
+ m_option_definition[2].usage_text = m_value_usage_text.data();
+}
+
+OptionGroupPythonClassWithDict::~OptionGroupPythonClassWithDict() {}
+
+Status OptionGroupPythonClassWithDict::SetOptionValue(
+ uint32_t option_idx,
+ llvm::StringRef option_arg,
+ ExecutionContext *execution_context) {
+ Status error;
+ const int short_option = m_option_definition[option_idx].short_option;
+ switch (option_idx) {
+ case 0: {
+ m_class_name.assign(option_arg);
+ } break;
+ case 1: {
+ if (m_current_key.empty())
+ m_current_key.assign(option_arg);
+ else
+ error.SetErrorStringWithFormat("Key: \"%s\" missing value.",
+ m_current_key.c_str());
+
+ } break;
+ case 2: {
+ if (!m_current_key.empty()) {
+ m_dict_sp->AddStringItem(m_current_key, option_arg);
+ m_current_key.clear();
+ }
+ else
+ error.SetErrorStringWithFormat("Value: \"%s\" missing matching key.",
+ option_arg.str().c_str());
+ } break;
+ default:
+ llvm_unreachable("Unimplemented option");
+ }
+ return error;
+}
+
+void OptionGroupPythonClassWithDict::OptionParsingStarting(
+ ExecutionContext *execution_context) {
+ m_current_key.erase();
+ m_dict_sp = std::make_shared<StructuredData::Dictionary>();
+}
+
+Status OptionGroupPythonClassWithDict::OptionParsingFinished(
+ ExecutionContext *execution_context) {
+ Status error;
+ // If we get here and there's contents in the m_current_key, somebody must
+ // have provided a key but no value.
+ if (!m_current_key.empty())
+ error.SetErrorStringWithFormat("Key: \"%s\" missing value.",
+ m_current_key.c_str());
+ return error;
+}
+
Modified: lldb/trunk/source/Interpreter/Options.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Options.cpp?rev=373673&r1=373672&r2=373673&view=diff
==============================================================================
--- lldb/trunk/source/Interpreter/Options.cpp (original)
+++ lldb/trunk/source/Interpreter/Options.cpp Thu Oct 3 15:18:51 2019
@@ -1380,6 +1380,10 @@ llvm::Expected<Args> Options::Parse(cons
? nullptr
: OptionParser::GetOptionArgument(),
execution_context);
+ // If the Option setting returned an error, we should stop parsing
+ // and return the error.
+ if (error.Fail())
+ break;
} else {
error.SetErrorStringWithFormat("invalid option with value '%i'", val);
}
More information about the lldb-commits
mailing list