- Prefer llvm::StringRef::find over allocating a std::string and then discarding it after calling the equivalent find method.
- improve readability of function iterator, and make the function variable a const ref.
- Prefer passing StringRef directly to ConstString::setString(StringRef &) over allocating a new std::string from a StringRef only to convert to const char *, which is then passed to ConstString::setCString(const char *)
Other than that is looks fine.
Don't make this kind of change, please. As long as the arguments fit in 120 characters we don't have a rule one way or the other about how to write argument lists like this. But changing them just because they look better to you results in unnecessary churn. Moreover, this is changing it away from the way all the other functions in this source file are written, so it ends up looking odd.
This change is made by clang-fomat using the rules in the lldb .clang-format file.
I'm willing to revert this part of the commit, but seeing as this change is essentially a refactoring of the whole method, it feels natural to also format the prototype while I’m at it.
lgtm other than nitpick on formatting. And sorry for delayed review - been sick.
Please just keep the space before the params and the return type on a separate line. Sadly, we can't use clang-format for function decls/defs in lldb because it doesn't support the lldb-style here, so the formatting of this must be done manually :(