As before, some copies are removed, a few are introduced. When I get to Option::SetValue, those that are introduced should go away.
Details
Diff Detail
Event Timeline
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.
source/Commands/CommandObjectRegister.cpp | ||
---|---|---|
269 | Add a StringRef variant as inside of OptionValueUInt64 it can use the StringRef::getAsInteger(). | |
source/Interpreter/OptionGroupVariable.cpp | ||
104 | Please add llvm::StringRef version of SetCurrentValue to OptionValueString: Error OptionValueString::SetCurrentValue(const llvm::StringRef &value); | |
107 | Please add llvm::StringRef version of SetCurrentValue to OptionValueString: |
I am not saying we have to do the printf changes, I was just seeing what you think. I would like to see the StringRef variants of functions put in as part of this.
Yeah, don't do any calls that don't need to be converted. Just the ones you need. Should just add 2 StringRef variant functions. Don't feel the need to completely fix OptionValueString or OptionValueUInt64. We can do the full change over in a future CL. See if you can just add the two quick ones.
You can still leave the "const char *" versions in there for now until you get to the cleanup. No spiral if you do that.
Updated the two requested functions. I still had to make a few more changes to catch the implicit conversions into each of these functions, but it wasn't too bad.
Add a StringRef variant as inside of OptionValueUInt64 it can use the StringRef::getAsInteger().