[Lldb-commits] [PATCH] D24847: Make OptionGroup::SetValue take a StringRef

Greg Clayton via lldb-commits lldb-commits at lists.llvm.org
Thu Sep 22 15:14:50 PDT 2016

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

We could avoid many of the copies in the printf statements by by doing something like this in a common header file:

  #define LLVM_STRINGREF_PRINTF_ARGS(s) (int)s.size(), s.data()

Then when doing errors or anything that does printf:

  error.SetErrorStringWithFormat("invalid all-threads value setting: \"" LLVM_STRINGREF_PRINTF_FORMAT "\"", LLVM_STRINGREF_PRINTF_ARGS(option_arg));

There are a few places where we should plumb through StringRef variants of calls, only 2, so I think it is worth including in this change.

Comment at: source/Commands/CommandObjectRegister.cpp:269
@@ -268,3 +268,3 @@
       case 's': {
-        OptionValueSP value_sp(OptionValueUInt64::Create(option_value, error));
+        OptionValueSP value_sp(OptionValueUInt64::Create(option_value.str().c_str(), error));
         if (value_sp)
Add a StringRef variant as inside of OptionValueUInt64 it can use the StringRef::getAsInteger().

Comment at: source/Interpreter/OptionGroupVariable.cpp:104
@@ -103,3 +103,3 @@
   case 'y':
-    error = summary.SetCurrentValue(option_arg);
+    error = summary.SetCurrentValue(option_arg.str().c_str());
Please add llvm::StringRef version of SetCurrentValue to OptionValueString:

    OptionValueString::SetCurrentValue(const llvm::StringRef &value);

Comment at: source/Interpreter/OptionGroupVariable.cpp:107
@@ -106,3 +106,3 @@
   case 'z':
-    error = summary_string.SetCurrentValue(option_arg);
+    error = summary_string.SetCurrentValue(option_arg.str().c_str());
Please add llvm::StringRef version of SetCurrentValue to OptionValueString:


More information about the lldb-commits mailing list