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

Yitzhak Mandelbaum via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 17 07:17:44 PDT 2021


ymandel added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:56
+    if (Name.find("::") != std::string::npos) {
+      return llvm::Regex(Name).match(Node.getQualifiedNameAsString());
+    }
----------------
flx wrote:
> ymandel wrote:
> > nit: while this change is not much worse than the existing code, the matcher really should perform this test for "::", and create the Regex, for each string just once. Regex construction is relatively expensive, and matchers of this sort (very generic) are often called quite frequently.
> > 
> > for that matter, this matcher is now really close to `matchesName` and would probably be best as a first-class matcher, rather than limited to clang-tidy/utils. But, I'll admit that such is beyond the scope of this patch.
> It wouldn't be hard to change it something like this:
> 
> 
> ```
> struct MatcherPair {                                                                                                       
>     llvm::Regex regex;                                                                                                       
>     bool matchQualifiedName = false;                                                                                         
>   };                                                                                                                         
>   std::vector<MatcherPair> Matchers;                                                                                         
>   std::transform(                                                                                                            
>       NameList.begin(), NameList.end(), std::back_inserter(Matchers),                                                        
>       [](const std::string& Name) {                                                                                          
>         return MatcherPair{                                                                                                  
>             .regex = llvm::Regex(Name),                                                                                      
>             .matchQualifiedName = Name.find("::") != std::string::npos,                                                      
>         };                                                                                                                   
>       });                                                                                                                    
>   return llvm::any_of(Matchers, [&Node](const MatcherPair& Matcher) {                                                        
>     if (Matcher.matchQualifiedName) {                                                                                        
>       return Matcher.regex.match(Node.getQualifiedNameAsString());                                                           
>     }                                                                                                                        
>     return Matcher.regex.match(Node.getName());                                                                              
>   });                       
> ```
> 
> Is this what you had in mind?
Thanks, that's almost it. The key point is that the code before the `return` be executed once, during matcher construction, and not each time `match` is called. You could define your own class (instead of using the macro) or just your own factory function:

```
struct MatcherPair {                                                                                                       
    llvm::Regex regex;                                                                                                       
    bool matchQualifiedName = false;                                                                                         
};        

AST_MATCHER_P(NamedDecl, matchesAnyListedNameImpl, std::vector<MatcherPair>, Matchers) {
  return llvm::any_of(Matchers, [&Node](const MatcherPair& Matcher) {                                                        
    if (Matcher.matchQualifiedName) {                                                                                        
      return Matcher.regex.match(Node.getQualifiedNameAsString());                                                           
    }                                                                                                                        
    return Matcher.regex.match(Node.getName());                                                                              
  });
}

Matcher<NamedDecl> matchesAnyListedName(std::vector<std::string> NameList) {
  std::vector<MatcherPair> Matchers;                                                                                         
  std::transform(                                                                                                            
      NameList.begin(), NameList.end(), std::back_inserter(Matchers),                                                        
      [](const std::string& Name) {                                                                                          
        return MatcherPair{                                                                                                  
            .regex = llvm::Regex(Name),                                                                                      
            .matchQualifiedName = Name.find("::") != std::string::npos,                                                      
        };                                                                                                                   
      });
  return matchesAnyListNameImpl(std::move(Matchers));
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98738



More information about the cfe-commits mailing list