[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