zturner removed rL LLVM as the repository for this revision. zturner updated this revision to Diff 81625. zturner added a comment.
Re-upload the correct diff. https://reviews.llvm.org/D27780 Files: lldb/include/lldb/Interpreter/Options.h lldb/include/lldb/lldb-private-types.h lldb/source/Commands/CommandObjectSettings.cpp lldb/source/Host/common/OptionParser.cpp lldb/source/Interpreter/Args.cpp lldb/source/Interpreter/OptionGroupOutputFile.cpp lldb/source/Interpreter/Options.cpp
Index: lldb/source/Interpreter/Options.cpp =================================================================== --- lldb/source/Interpreter/Options.cpp +++ lldb/source/Interpreter/Options.cpp @@ -333,27 +333,14 @@ } } -bool Options::SupportsLongOption(const char *long_option) { - if (!long_option || !long_option[0]) +bool Options::SupportsLongOption(llvm::StringRef long_option) { + long_option.consume_front("--"); + if (long_option.empty()) return false; - auto opt_defs = GetDefinitions(); - if (opt_defs.empty()) - return false; - - const char *long_option_name = long_option; - if (long_option[0] == '-' && long_option[1] == '-') - long_option_name += 2; - - for (auto &def : opt_defs) { - if (!def.long_option) - continue; - - if (strcmp(def.long_option, long_option_name) == 0) - return true; - } - - return false; + return llvm::any_of(GetDefinitions(), [=](const OptionDefinition &D) { + return D.long_option == long_option; + }); } enum OptionDisplayType { @@ -603,9 +590,10 @@ strm.IndentMore(5); - if (opt_defs[i].usage_text) + if (!opt_defs[i].usage_text.empty()) OutputFormattedUsageText(strm, opt_defs[i], screen_width); - if (opt_defs[i].enum_values != nullptr) { + + if (opt_defs[i].enum_values == nullptr) { strm.Indent(); strm.Printf("Values: "); for (int k = 0; opt_defs[i].enum_values[k].string_value != nullptr; @@ -670,10 +658,6 @@ auto opt_defs = GetDefinitions(); - std::string cur_opt_std_str(input.GetArgumentAtIndex(cursor_index)); - cur_opt_std_str.erase(char_pos); - const char *cur_opt_str = cur_opt_std_str.c_str(); - for (size_t i = 0; i < opt_element_vector.size(); i++) { int opt_pos = opt_element_vector[i].opt_pos; int opt_arg_pos = opt_element_vector[i].opt_arg_pos; @@ -708,18 +692,17 @@ } return true; } else if (opt_defs_index != OptionArgElement::eUnrecognizedArg) { + auto cur_opt_str = input[cursor_index].ref.take_front(char_pos); + // We recognized it, if it an incomplete long option, complete it anyway - // (getopt_long_only is - // happy with shortest unique string, but it's still a nice thing to - // do.) Otherwise return - // The string so the upper level code will know this is a full match and - // add the " ". - if (cur_opt_str && strlen(cur_opt_str) > 2 && cur_opt_str[0] == '-' && - cur_opt_str[1] == '-' && - strcmp(opt_defs[opt_defs_index].long_option, cur_opt_str) != 0) { + // (getopt_long_only is happy with shortest unique string, but it's + // still a nice thing to do.) Otherwise return the string so the upper + // level code will know this is a full match and add the " ". + if (cur_opt_str.startswith("--") && + cur_opt_str.drop_front(2) != opt_defs[opt_defs_index].long_option) { std::string full_name("--"); full_name.append(opt_defs[opt_defs_index].long_option); - matches.AppendString(full_name.c_str()); + matches.AppendString(full_name); return true; } else { matches.AppendString(input.GetArgumentAtIndex(cursor_index)); @@ -729,33 +712,32 @@ // FIXME - not handling wrong options yet: // Check to see if they are writing a long option & complete it. // I think we will only get in here if the long option table has two - // elements - // that are not unique up to this point. getopt_long_only does shortest - // unique match - // for long options already. - - if (cur_opt_str && strlen(cur_opt_str) > 2 && cur_opt_str[0] == '-' && - cur_opt_str[1] == '-') { - for (auto &def : opt_defs) { - if (!def.long_option) - continue; + // elements that are not unique up to this point. getopt_long_only + // does shortest unique match for long options already. + auto cur_opt_str = input[cursor_index].ref.take_front(char_pos); - if (strstr(def.long_option, cur_opt_str + 2) == def.long_option) { - std::string full_name("--"); - full_name.append(def.long_option); - // The options definitions table has duplicates because of the - // way the grouping information is stored, so only add once. - bool duplicate = false; - for (size_t k = 0; k < matches.GetSize(); k++) { - if (matches.GetStringAtIndex(k) == full_name) { - duplicate = true; - break; - } - } - if (!duplicate) - matches.AppendString(full_name.c_str()); + if (!cur_opt_str.consume_front("--")) + return true; + + auto range = llvm::make_filter_range( + opt_defs, [cur_opt_str](const OptionDefinition &D) { + return D.long_option == cur_opt_str; + }); + + for (auto &def : range) { + std::string full_name("--"); + full_name.append(def.long_option); + // The options definitions table has duplicates because of the way the + // grouping information is stored, so only add once. + bool duplicate = false; + for (size_t k = 0; k < matches.GetSize(); k++) { + if (matches.GetStringAtIndex(k) == full_name) { + duplicate = true; + break; } } + if (!duplicate) + matches.AppendString(full_name); } return true; } @@ -845,14 +827,13 @@ continue; int cur_arg_pos = opt_element_vector[i].opt_arg_pos; - const char *cur_opt_name = opt_defs[cur_defs_index].long_option; + auto cur_opt_name = opt_defs[cur_defs_index].long_option; // If this is the "shlib" option and there was an argument provided, // restrict it to that shared library. - if (cur_opt_name && strcmp(cur_opt_name, "shlib") == 0 && - cur_arg_pos != -1) { - const char *module_name = input.GetArgumentAtIndex(cur_arg_pos); - if (module_name) { + if (cur_opt_name == "shlib" && cur_arg_pos != -1) { + auto module_name = input[cur_arg_pos].ref; + if (!module_name.empty()) { FileSpec module_spec(module_name, false); lldb::TargetSP target_sp = interpreter.GetDebugger().GetSelectedTarget(); Index: lldb/source/Interpreter/OptionGroupOutputFile.cpp =================================================================== --- lldb/source/Interpreter/OptionGroupOutputFile.cpp +++ lldb/source/Interpreter/OptionGroupOutputFile.cpp @@ -23,15 +23,17 @@ OptionGroupOutputFile::~OptionGroupOutputFile() {} -static const uint32_t SHORT_OPTION_APND = 0x61706e64; // 'apnd' +static const int SHORT_OPTION_APND = 0x61706e64; // 'apnd' -static OptionDefinition g_option_table[] = { - {LLDB_OPT_SET_1, false, "outfile", 'o', OptionParser::eRequiredArgument, +using L = llvm::StringLiteral; + +static constexpr OptionDefinition g_option_table[] = { + {LLDB_OPT_SET_1, false, L("outfile"), 'o', OptionParser::eRequiredArgument, nullptr, nullptr, 0, eArgTypeFilename, - "Specify a path for capturing command output."}, - {LLDB_OPT_SET_1, false, "append-outfile", SHORT_OPTION_APND, + L("Specify a path for capturing command output.")}, + {LLDB_OPT_SET_1, false, L("append-outfile"), SHORT_OPTION_APND, OptionParser::eNoArgument, nullptr, nullptr, 0, eArgTypeNone, - "Append to the file specified with '--outfile <path>'."}, + L("Append to the file specified with '--outfile <path>'.")}, }; llvm::ArrayRef<OptionDefinition> OptionGroupOutputFile::GetDefinitions() { Index: lldb/source/Interpreter/Args.cpp =================================================================== --- lldb/source/Interpreter/Args.cpp +++ lldb/source/Interpreter/Args.cpp @@ -26,9 +26,12 @@ #include "lldb/Target/Target.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringSwitch.h" +#include "llvm/Support/FormatVariadic.h" + using namespace lldb; using namespace lldb_private; @@ -947,12 +950,10 @@ size_t Args::FindArgumentIndexForOption(Option *long_options, int long_options_index) const { - char short_buffer[3]; - char long_buffer[255]; - ::snprintf(short_buffer, sizeof(short_buffer), "-%c", - long_options[long_options_index].val); - ::snprintf(long_buffer, sizeof(long_buffer), "--%s", - long_options[long_options_index].definition->long_option); + llvm::SmallString<3> short_buffer = + llvm::formatv("-{0}", (char)long_options[long_options_index].val); + llvm::SmallString<255> long_buffer = llvm::formatv( + "--{0}", long_options[long_options_index].definition->long_option); for (auto entry : llvm::enumerate(m_entries)) { if (entry.Value.ref.startswith(short_buffer) || Index: lldb/source/Host/common/OptionParser.cpp =================================================================== --- lldb/source/Host/common/OptionParser.cpp +++ lldb/source/Host/common/OptionParser.cpp @@ -11,6 +11,9 @@ #include "lldb/Host/HostGetOpt.h" #include "lldb/lldb-private-types.h" +#include "llvm/Support/Allocator.h" +#include "llvm/Support/StringSaver.h" + #include <vector> using namespace lldb_private; @@ -31,11 +34,14 @@ int OptionParser::Parse(int argc, char *const argv[], llvm::StringRef optstring, const Option *longopts, int *longindex) { std::vector<option> opts; + llvm::BumpPtrAllocator allocator; + llvm::StringSaver saver(allocator); + while (longopts->definition != nullptr) { option opt; opt.flag = longopts->flag; opt.val = longopts->val; - opt.name = longopts->definition->long_option; + opt.name = saver.save(longopts->definition->long_option).data(); opt.has_arg = longopts->definition->option_has_arg; opts.push_back(opt); ++longopts; Index: lldb/source/Commands/CommandObjectSettings.cpp =================================================================== --- lldb/source/Commands/CommandObjectSettings.cpp +++ lldb/source/Commands/CommandObjectSettings.cpp @@ -27,9 +27,11 @@ // CommandObjectSettingsSet //------------------------------------------------------------------------- -static OptionDefinition g_settings_set_options[] = { +using L = llvm::StringLiteral; + +static constexpr OptionDefinition g_settings_set_options[] = { // clang-format off - { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, nullptr, 0, eArgTypeNone, "Apply the new value to the global default value." } + { LLDB_OPT_SET_2, false, L("global"), 'g', OptionParser::eNoArgument, nullptr, nullptr, 0, eArgTypeNone, L("Apply the new value to the global default value.") } // clang-format on }; Index: lldb/include/lldb/lldb-private-types.h =================================================================== --- lldb/include/lldb/lldb-private-types.h +++ lldb/include/lldb/lldb-private-types.h @@ -104,22 +104,51 @@ }; struct OptionDefinition { - uint32_t usage_mask; // Used to mark options that can be used together. If (1 - // << n & usage_mask) != 0 - // then this option belongs to option set n. - bool required; // This option is required (in the current usage level) - const char *long_option; // Full name for this option. - int short_option; // Single character for this option. - int option_has_arg; // no_argument, required_argument or optional_argument - OptionValidator *validator; // If non-NULL, option is valid iff - // |validator->IsValid()|, otherwise always valid. - OptionEnumValueElement *enum_values; // If non-NULL an array of enum values. - uint32_t completion_type; // Cookie the option class can use to do define the - // argument completion. - lldb::CommandArgumentType argument_type; // Type of argument this option takes - const char *usage_text; // Full text explaining what this options does and - // what (if any) argument to - // pass it. + OptionDefinition() {} + + constexpr OptionDefinition(uint32_t mask, bool required, + llvm::StringRef long_opt, int short_opt, + int has_arg, OptionValidator *validator, + OptionEnumValueElement *enum_values, + uint32_t completion_type, + lldb::CommandArgumentType arg_type, + llvm::StringRef usage) + : usage_mask(mask), required(required), long_option(long_opt), + short_option(short_opt), option_has_arg(has_arg), validator(validator), + enum_values(enum_values), completion_type(completion_type), + argument_type(arg_type), usage_text(usage_text) {} + + // Used to mark options that can be used together. If + // (1 << n & usage_mask) != 0 then this option belongs to option set n. + uint32_t usage_mask = 0; + + // This option is required (in the current usage level) + bool required = false; + + // Full name for this option. + llvm::StringRef long_option; + + // Single character for this option. + int short_option = 0; + + // no_argument, required_argument or optional_argument + int option_has_arg = 0; + + // If non-NULL, option is valid iff |validator->IsValid()|, otherwise always + // valid. + OptionValidator *validator = nullptr; + + // If non-NULL an array of enum values. + OptionEnumValueElement *enum_values = nullptr; + + // Cookie the option class can use to do define the argument completion. + uint32_t completion_type = 0; + + // Type of argument this option takes + lldb::CommandArgumentType argument_type = lldb::eArgTypeNone; + + // Full text explaining what this options does and what argument(s) it takes. + llvm::StringRef usage_text; }; typedef struct type128 { uint64_t x[2]; } type128; Index: lldb/include/lldb/Interpreter/Options.h =================================================================== --- lldb/include/lldb/Interpreter/Options.h +++ lldb/include/lldb/Interpreter/Options.h @@ -155,7 +155,7 @@ void GenerateOptionUsage(Stream &strm, CommandObject *cmd, uint32_t screen_width); - bool SupportsLongOption(const char *long_option); + bool SupportsLongOption(llvm::StringRef long_option); // The following two pure virtual functions must be defined by every // class that inherits from this class.
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits