[PATCH] D51360: [clang-tidy] Use simple string matching instead of Regex
Haojian Wu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 30 05:18:56 PDT 2018
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
More information about the cfe-commits
mailing list