[Lldb-commits] [lldb] r282079 - Make lldb::Regex use StringRef.
Zachary Turner via lldb-commits
lldb-commits at lists.llvm.org
Wed Sep 21 17:14:34 PDT 2016
On Wed, Sep 21, 2016 at 5:00 PM Greg Clayton <gclayton at apple.com> wrote:
> Please submit a change requests when doing these kinds of things and let
> people comment on the changes before committing such things.
> We deleted functions that were correctly using "const char *" like:
> bool Execute(llvm::StringRef string, Match *match = nullptr) const;
> bool Execute(const char *, Match * = nullptr) = delete;
> Yet inside these functions you do "string.str().c_str()" because these
> need to be null terminated? this indicates that StringRef is NOT the
> correct choice here. You will make heap copy of all strings (if they are
> long enough and regex matching string can be very long) that you compile or
> execute. We already make a copy of the string when we compile, so StringRef
> didn't really save us anything on compile and execute is always making a
> copy just to run the expression on. I am fine with both being there in case
> one might be more efficient, but taking them out just to use a less
> efficient version that uses llvm::StringRef is not the kind of changes we
> should be doing all over.
> We will make copies of all strings in all of the following changes:
> - Unneeded copy:
> - new TypeNameSpecifierImpl(regex->GetText(), true));
> + new TypeNameSpecifierImpl(regex->GetText().str().c_str(), true));
All of these copies are only temporary. As I've said over and over, once
everything is StringRef all the way down, the copies all disappear. It's
only a copy because TypeNameSpecifierImpl doesn't take a STringRef, and I
didn't want to change the whole codebase all at once, but rather
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the lldb-commits