On Wed, Sep 21, 2016 at 5:00 PM Greg Clayton <gclay...@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 incrementally.
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
