hokein added inline comments.
================ Comment at: clang-tidy/abseil/AbseilMatcher.h:16 + +/// Matches AST nodes that were expanded within abseil-header-files. +AST_POLYMORPHIC_MATCHER(isExpansionNotInAbseilHeader, ---------------- nit: We need proper documentation for this matcher, since it is exposed to users. ================ Comment at: clang-tidy/abseil/AbseilMatcher.h:17 +/// Matches AST nodes that were expanded within abseil-header-files. +AST_POLYMORPHIC_MATCHER(isExpansionNotInAbseilHeader, + AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc, ---------------- - using negative for AST matcher seems uncommon in ASTMatchers, for negative we have `unless`, so I'd suggest implementing `isInAbseilHeader`. - it seems that this matcher is very similar to the matcher in https://reviews.llvm.org/D50542. I think `isInAbseilFile` should also cover your case here, and also ignore the warning if you run the check on abseil core files. ================ Comment at: clang-tidy/abseil/AbseilMatcher.h:32 + auto Filename = FileEntry->getName(); + llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|" + "synchronization|types|utiliy)"); ---------------- The regex seems incomplete, we are missing `algorithm`, `time` subdirectory. ================ Comment at: clang-tidy/abseil/AbseilMatcher.h:33 + llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|" + "synchronization|types|utiliy)"); + return (!RE.match(Filename)); ---------------- typo: `utility` ================ Comment at: clang-tidy/abseil/AbseilMatcher.h:34 + "synchronization|types|utiliy)"); + return (!RE.match(Filename)); +} ---------------- nit: remove the outer `()`. https://reviews.llvm.org/D50580 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
