[clang-tidy] Use simple string matching instead of Regex

[kbobyrev]

Summary

Instead of parsing and compiling the llvm::Regex each time, it's faster to use basic string matching for filename prefix check.

[hokein]

Thanks very much for improving it!

clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h

45 ↗

(On Diff #162875)

The logic is a bit subtle here. Agree that using regex is overkill.

We still want substring match here, since the Filename does not always start with absl/, so your improvement will change the behavior. I think we should do absl/[absl-libraries] substring match.

[kbobyrev]

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?

[hokein]

In 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.

clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h

43 ↗

(On Diff #163302)

I think we should have a comment explaining we are matching absl/[absl-libraries] -- the current code is not as straightforward as before.

[kbobyrev]

Mention that the code actually performs absl/(any|absl|library) substring match.

clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h

 //===- AbseilMatcher.h - clang-tidy ---------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
-//
+
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <algorithm>
 
 namespace clang {
 namespace ast_matchers {
 
 /// Matches AST nodes that were found within Abseil files.
 ///
 /// Example matches Y but not X
 ///     (matcher = cxxRecordDecl(isInAbseilFile())
 /// \code
 ///   #include "absl/strings/internal-file.h"
 ///   class X {};
 /// \endcode
 /// absl/strings/internal-file.h:
 /// \code
 ///   class Y {};
 /// \endcode
 ///
 /// Usable as: Matcher<Decl>, Matcher<Stmt>, Matcher<TypeLoc>,
 /// Matcher<NestedNameSpecifierLoc>
-
-AST_POLYMORPHIC_MATCHER(isInAbseilFile,
-                        AST_POLYMORPHIC_SUPPORTED_TYPES(
-                            Decl, Stmt, TypeLoc, NestedNameSpecifierLoc)) {
+AST_POLYMORPHIC_MATCHER(
+    isInAbseilFile, AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc,
+                                                    NestedNameSpecifierLoc)) {
   auto &SourceManager = Finder->getASTContext().getSourceManager();
   SourceLocation Loc = Node.getBeginLoc();
   if (Loc.isInvalid())
     return false;
   const FileEntry *FileEntry =
       SourceManager.getFileEntryForID(SourceManager.getFileID(Loc));
   if (!FileEntry)
     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);
+  // Determine whether filepath contains "absl/[absl-library]" substring, where
+  // [absl-library] is AbseilLibraries list entry.
+  StringRef Path = FileEntry->getName();
+  const static llvm::SmallString<5> AbslPrefix("absl/");
+  size_t PrefixPosition = Path.find(AbslPrefix);
+  if (PrefixPosition == StringRef::npos)
+    return false;
+  Path = Path.drop_front(PrefixPosition + AbslPrefix.size());
+  static const char *AbseilLibraries[] = {
+      "algorithm",       "base", "container", "debugging",
+      "memory",          "meta", "numeric",   "strings",
+      "synchronization", "time", "types",     "utility"};
+  return std::any_of(
+      std::begin(AbseilLibraries), std::end(AbseilLibraries),
+      [&](const char *Library) { return Path.startswith(Library); });
 }
 
 } // namespace ast_matchers
 } // namespace clang