hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land.
In https://reviews.llvm.org/D51360#1219024, @kbobyrev wrote: > Ah, correct. I totally forgot about the `^string$` for the exact match. > > This should not change behavior now. I believe what you propose would make > the code easier (there'd only be `AbseilLibraries = {"absl/algorithm", ...}` > and the final `return std::any_of(..., [](...) { return Path.find(Library) != > StringRef::npos; });` but the performance would be worse (because of the > presence of the `absl/` prefix in each entry). Would you consider this less > efficient but easier-to-follow approach better? It was my suggestion. I'm fine with the current way as long as we have a clear comment explaining it. ================ Comment at: clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h:43 return false; - StringRef Filename = FileEntry->getName(); - llvm::Regex RE( - "absl/(algorithm|base|container|debugging|memory|meta|numeric|strings|" - "synchronization|time|types|utility)"); - return RE.match(Filename); + StringRef Path = FileEntry->getName(); + const static llvm::SmallString<5> AbslPrefix("absl/"); ---------------- I think we should have a comment explaining we are matching `absl/[absl-libraries]` -- the current code is not as straightforward as before. https://reviews.llvm.org/D51360 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
