This revision was automatically updated to reflect the committed changes.
Closed by commit rL365665: Options: Reduce code duplication (authored by
labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63770/new/
https://reviews.llvm.org/D63770
Files:
lldb/trunk/include/lldb/Host/OptionParser.h
lldb/trunk/source/Host/common/OptionParser.cpp
lldb/trunk/source/Interpreter/CommandAlias.cpp
lldb/trunk/source/Interpreter/Options.cpp
Index: lldb/trunk/source/Interpreter/CommandAlias.cpp
===================================================================
--- lldb/trunk/source/Interpreter/CommandAlias.cpp
+++ lldb/trunk/source/Interpreter/CommandAlias.cpp
@@ -30,6 +30,8 @@
Args args(options_args);
std::string options_string(options_args);
+ // TODO: Find a way to propagate errors in this CommandReturnObject up the
+ // stack.
CommandReturnObject result;
// Check to see if the command being aliased can take any command options.
Options *options = cmd_obj_sp->GetOptions();
Index: lldb/trunk/source/Interpreter/Options.cpp
===================================================================
--- lldb/trunk/source/Interpreter/Options.cpp
+++ lldb/trunk/source/Interpreter/Options.cpp
@@ -930,6 +930,7 @@
result.push_back(const_cast<char *>("<FAKE-ARG0>"));
for (const Args::ArgEntry &entry : args)
result.push_back(const_cast<char *>(entry.c_str()));
+ result.push_back(nullptr);
return result;
}
@@ -972,19 +973,15 @@
return size_t(-1);
}
-llvm::Expected<Args> Options::ParseAlias(const Args &args,
- OptionArgVector *option_arg_vector,
- std::string &input_line) {
- StreamString sstr;
- int i;
- Option *long_options = GetLongOptions();
+static std::string BuildShortOptions(const Option *long_options) {
+ std::string storage;
+ llvm::raw_string_ostream sstr(storage);
- if (long_options == nullptr) {
- return llvm::make_error<llvm::StringError>("Invalid long options",
- llvm::inconvertibleErrorCode());
- }
+ // Leading : tells getopt to return a : for a missing option argument AND to
+ // suppress error messages.
+ sstr << ":";
- for (i = 0; long_options[i].definition != nullptr; ++i) {
+ for (size_t i = 0; long_options[i].definition != nullptr; ++i) {
if (long_options[i].flag == nullptr) {
sstr << (char)long_options[i].val;
switch (long_options[i].definition->option_has_arg) {
@@ -1000,6 +997,20 @@
}
}
}
+ return std::move(sstr.str());
+}
+
+llvm::Expected<Args> Options::ParseAlias(const Args &args,
+ OptionArgVector *option_arg_vector,
+ std::string &input_line) {
+ Option *long_options = GetLongOptions();
+
+ if (long_options == nullptr) {
+ return llvm::make_error<llvm::StringError>("Invalid long options",
+ llvm::inconvertibleErrorCode());
+ }
+
+ std::string short_options = BuildShortOptions(long_options);
Args args_copy = args;
std::vector<char *> argv = GetArgvForParsing(args);
@@ -1009,8 +1020,13 @@
int val;
while (true) {
int long_options_index = -1;
- val = OptionParser::Parse(argv.size(), &*argv.begin(), sstr.GetString(),
- long_options, &long_options_index);
+ val = OptionParser::Parse(argv, short_options, long_options,
+ &long_options_index);
+
+ if (val == ':') {
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "last option requires an argument");
+ }
if (val == -1)
break;
@@ -1116,33 +1132,13 @@
OptionElementVector Options::ParseForCompletion(const Args &args,
uint32_t cursor_index) {
OptionElementVector option_element_vector;
- StreamString sstr;
Option *long_options = GetLongOptions();
option_element_vector.clear();
if (long_options == nullptr)
return option_element_vector;
- // Leading : tells getopt to return a : for a missing option argument AND to
- // suppress error messages.
-
- sstr << ":";
- for (int i = 0; long_options[i].definition != nullptr; ++i) {
- if (long_options[i].flag == nullptr) {
- sstr << (char)long_options[i].val;
- switch (long_options[i].definition->option_has_arg) {
- default:
- case OptionParser::eNoArgument:
- break;
- case OptionParser::eRequiredArgument:
- sstr << ":";
- break;
- case OptionParser::eOptionalArgument:
- sstr << "::";
- break;
- }
- }
- }
+ std::string short_options = BuildShortOptions(long_options);
std::unique_lock<std::mutex> lock;
OptionParser::Prepare(lock);
@@ -1153,10 +1149,6 @@
std::vector<char *> dummy_vec = GetArgvForParsing(args);
- // I stick an element on the end of the input, because if the last element
- // is option that requires an argument, getopt_long_only will freak out.
- dummy_vec.push_back(const_cast<char *>("<FAKE-VALUE>"));
-
bool failed_once = false;
uint32_t dash_dash_pos = -1;
@@ -1164,8 +1156,8 @@
bool missing_argument = false;
int long_options_index = -1;
- val = OptionParser::Parse(dummy_vec.size(), &dummy_vec[0], sstr.GetString(),
- long_options, &long_options_index);
+ val = OptionParser::Parse(dummy_vec, short_options, long_options,
+ &long_options_index);
if (val == -1) {
// When we're completing a "--" which is the last option on line,
@@ -1328,7 +1320,6 @@
ExecutionContext *execution_context,
lldb::PlatformSP platform_sp,
bool require_validation) {
- StreamString sstr;
Status error;
Option *long_options = GetLongOptions();
if (long_options == nullptr) {
@@ -1336,38 +1327,15 @@
llvm::inconvertibleErrorCode());
}
- // Leading : tells getopt to return a : for a missing option argument AND to
- // suppress error messages.
- sstr << ":";
- for (int i = 0; long_options[i].definition != nullptr; ++i) {
- if (long_options[i].flag == nullptr) {
- if (isprint8(long_options[i].val)) {
- sstr << (char)long_options[i].val;
- switch (long_options[i].definition->option_has_arg) {
- default:
- case OptionParser::eNoArgument:
- break;
- case OptionParser::eRequiredArgument:
- sstr << ':';
- break;
- case OptionParser::eOptionalArgument:
- sstr << "::";
- break;
- }
- }
- }
- }
+ std::string short_options = BuildShortOptions(long_options);
std::vector<char *> argv = GetArgvForParsing(args);
- // If the last option requires an argument but doesn't have one,
- // some implementations of getopt_long will still try to read it.
- argv.push_back(nullptr);
std::unique_lock<std::mutex> lock;
OptionParser::Prepare(lock);
int val;
while (true) {
int long_options_index = -1;
- val = OptionParser::Parse(argv.size() - 1, &*argv.begin(), sstr.GetString(),
- long_options, &long_options_index);
+ val = OptionParser::Parse(argv, short_options, long_options,
+ &long_options_index);
if (val == ':') {
error.SetErrorStringWithFormat("last option requires an argument");
Index: lldb/trunk/source/Host/common/OptionParser.cpp
===================================================================
--- lldb/trunk/source/Host/common/OptionParser.cpp
+++ lldb/trunk/source/Host/common/OptionParser.cpp
@@ -27,8 +27,9 @@
void OptionParser::EnableError(bool error) { opterr = error ? 1 : 0; }
-int OptionParser::Parse(int argc, char *const argv[], llvm::StringRef optstring,
- const Option *longopts, int *longindex) {
+int OptionParser::Parse(llvm::MutableArrayRef<char *> argv,
+ llvm::StringRef optstring, const Option *longopts,
+ int *longindex) {
std::vector<option> opts;
while (longopts->definition != nullptr) {
option opt;
@@ -41,7 +42,8 @@
}
opts.push_back(option());
std::string opt_cstr = optstring;
- return getopt_long_only(argc, argv, opt_cstr.c_str(), &opts[0], longindex);
+ return getopt_long_only(argv.size() - 1, argv.data(), opt_cstr.c_str(),
+ &opts[0], longindex);
}
char *OptionParser::GetOptionArgument() { return optarg; }
Index: lldb/trunk/include/lldb/Host/OptionParser.h
===================================================================
--- lldb/trunk/include/lldb/Host/OptionParser.h
+++ lldb/trunk/include/lldb/Host/OptionParser.h
@@ -13,6 +13,7 @@
#include <string>
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/ArrayRef.h"
struct option;
@@ -37,8 +38,11 @@
static void EnableError(bool error);
- static int Parse(int argc, char *const argv[], llvm::StringRef optstring,
- const Option *longopts, int *longindex);
+ /// Argv must be an argument vector "as passed to main", i.e. terminated with
+ /// a nullptr.
+ static int Parse(llvm::MutableArrayRef<char *> argv,
+ llvm::StringRef optstring, const Option *longopts,
+ int *longindex);
static char *GetOptionArgument();
static int GetOptionIndex();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
