JDevlieghere added a comment. This code review seems to address a few things. I think it might have been easier to discuss if they were separate patches, but here are my comments:
- Thanks for removing the stale inheritance! (LGTM) - Thanks for adding the relational operator back. I couldn't find why they were needed (because they weren't used if I understand D66398 <https://reviews.llvm.org/D66398> correctly). (LGTM) - While I was doing the transition I considered the lifetime and came to the conclusion that the regex input string (the StringRef) does not need to outlive the regex compilation (`regcomp`). The interface of `llvm::Regex` gives the same impression. Is this not correct? - I'm not a fan of the laziness. Do we really need it? What's the cost of compiling an empty regex for the default constructed object? Now we have to (lazily) recompile the regex after every move, even though an `llvm::Regex` was designed to be movable. ================ Comment at: lldb/include/lldb/Utility/RegularExpression.h:74 /// Return an error if the regular expression failed to compile. + /// The value is not valid if the object has been copy/move-ctored. llvm::Error GetError() const; ---------------- This doesn't sound right, we return the error by value and llvm::StringError stores the error string. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66392/new/ https://reviews.llvm.org/D66392 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
