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

Reply via email to