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

[Greg Clayton via lldb-commits]

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_FORMAT "%*s"
  #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());
     break;
----------------
Please add llvm::StringRef version of SetCurrentValue to OptionValueString:

```
    Error
    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());
     break;
----------------
Please add llvm::StringRef version of SetCurrentValue to OptionValueString:



https://reviews.llvm.org/D24847