[PATCH] D98738: [clang-tidy] performance-* checks: Match AllowedTypes against qualified type names when they contain "::".

[Felix Berger via Phabricator via cfe-commits]

flx marked 6 inline comments as done.
flx added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:52
 
 AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector<std::string>,
               NameList) {
----------------
hokein wrote:
> worth some comments on this.
Done in its new location.


================
Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:55
   return llvm::any_of(NameList, [&Node](const std::string &Name) {
-      return llvm::Regex(Name).match(Node.getName());
-    });
+    if (Name.find("::") != std::string::npos) {
+      return llvm::Regex(Name).match(Node.getQualifiedNameAsString());
----------------
hokein wrote:
> hokein wrote:
> > If we pass the name as a `llvm::StringRef`, we can use `if (Name.contains("::"))` which seems a bit clearer to me.
> there is a tricky case where the `::` is a prefix of the Name, e.g.  a global symbol x, the Name is `::x`, and `getQualifiedNameAsString` of symbol x is `x` (without the `::` prefix), then the matcher will not find a match, but we expect this is a match right?
> 
I agree that's a tricky case.

We could prepend "::" to getQualifiedNameAsString() and always match against it. Would this work?

Or we could do this only for regex names that have a leading "::".  This would avoid extra string operations, and users could still match using "^foo::Bar$" to match ::foo::Bar.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98738/new/

https://reviews.llvm.org/D98738