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); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits