[PATCH] D107323: [clang-tidy] Add skip-headers; use skipLocation and setTraversalScope
chh created this revision. Herald added subscribers: kbarton, xazax.hun, nemanjai. chh requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. [clang-tidy] Add skip-headers; use skipLocation and setTraversalScope - This depends on https://reviews.llvm.org/D104071. - This new feature has impacts similar to --header-filter and --system-headers. - Clang-tidy by default checks header files, reports the number of warnings found in header files but not those warning messages. - With --header-filter, clang-tidy shows warnings in selected header files, but not system header files. - With --system-headers, clang-tidy shows also warnings in system header files. - With --skip-headers, clang-tidy does not check header files, and does not know or count warnings in header files. However, if --header-filter or --system-headers are used, those header files will be checked and warnings be counted and displayed. - For users who do not need to know the number of not-shown warnings in header files, this feature saves maybe 60% to 80% of clang-tidy run-time, by skipping checks of header files. When building Android system with clang-tidy, repeated checks on commonly included header files in multiple compilation units took significant part of build time, which could be saved by this new feature. - Note that this is not the same as PCH, which usually saves parsing time, but not semantic checks or code generation time. - This part 1 implementation only affects the MatchFinder-based checks, not the PPCallback-based or the static analyzer checks. Follow up changes could make the other checks to skip header files and save more time. - Add a --show-all-warnings flag. It is hidden, not shown by --help. When it is used, ClangTidyDiagnosticConsumer::checkFilters will not suppress any diagnostic message. This is useful to find tidy checks that have not yet handled the --skip-headers flag. - Implementation Methods: - Add a shared LocFilter to Tidy's MatchFinderOptions. - In MatchASTVisitor::TraverseDecl, skip a DeclNode if the MatchFinderOptions has a LocFilter that decides to skip the DeclNode location. - LocFilter::skipLocation checks if a location should not be checked. - Use simple cache of the last skipped/accepted FileID in LocFilter to minimize the overhead of skipLocation. - Add an AllFileFinder for checks that need to match all source files. - ForwardDeclarationNamespaceCheck is registered as an AllFileCheck, to avoid missing valid warnings on main source files. - UnusedUsingDeclsCheck is registered as an AllFileCheck, to avoid incorrect warnings on main source files. - Their diag functions call ClangTidyCheck::skipLocation to skip diagnostics on header files. - Depending on new changes to set/getTraversalScope in https://reviews.llvm.org/D104071, the AST was modified to exclude header file top-level Decls during the call to MatchFinder's ASTConsumer's HandleTranslationUnit. - AllFileFinder's and static analyzer's AST consumers are still called with the unmodified full AST. - ClangTidyASTConsumer overrides two member functions of MultiplexConsumer and accesses the Consumers data member. - in HandleTopLevelDecl to collect filtered Decls. - in HandleTranslationUnit to call get/setTraversalScope before calling the Finder ASTConcumser, which calls matchAST. - Tests: - Add skip-headers*.cpp tests to show the effects of new flags. - Add bugprone-forward-declaration-namespace-header.cpp test to show how AllFileMatchFinder-based checks can depend on header Decls. - Add new test cases into misc-unused-using-decls.cpp. - Extend some clang-tidy checker tests to make sure that --skip-headers skips checks/warnings in the header files: google-namespaces.cpp modernize-pass-by-value-header.cpp - Add runs with --skip-headers in some clang-tidy checker tests to make sure that the option does not hide warnings in the main file: - about 18 other clang-tools-extra/test/clang-tidy/checkers/*.cpp - Some tests were added --skip-headers=0 flag to get expected warning messages in test header files: checkers/abseil-no-internal-dependencies.cpp checkers/abseil-upgrade-duration-conversions.cpp checkers/google-namespaces.cpp infrastructure/file-filter-symlinks.cpp infrastructure/file-filter.cpp infrastructure/line-filter.cpp https://reviews.llvm.org/D107323 Files: clang-tools-extra/clang-tidy/ClangTidy.cpp clang-tools-extra/clang-tidy/ClangTidyCheck.cpp clang-tools-extra/clang-tidy/ClangTidyCheck.h clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp clang-tools-extra/clang-tidy/ClangTidyModule.cpp clang-tools-extra/clang-tidy/ClangTidyModule.h clang-tools-extra/clang-tidy/ClangTidyOptions.cpp clang-tools-extra/clang-tidy/ClangTidyOptions.h clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespace
[PATCH] D107323: [clang-tidy] Add skip-headers; use skipLocation and setTraversalScope
chh updated this revision to Diff 367120. chh edited the summary of this revision. chh added a comment. Factor out LocationFilter; improve more tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107323/new/ https://reviews.llvm.org/D107323 Files: clang-tools-extra/clang-tidy/ClangTidy.cpp clang-tools-extra/clang-tidy/ClangTidyCheck.h clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h clang-tools-extra/clang-tidy/ClangTidyModule.cpp clang-tools-extra/clang-tidy/ClangTidyModule.h clang-tools-extra/clang-tidy/ClangTidyOptions.cpp clang-tools-extra/clang-tidy/ClangTidyOptions.h clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-forward-declaration-namespace/a.h clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-forward-declaration-namespace/b.h clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-pass-by-value/header.h clang-tools-extra/test/clang-tidy/checkers/Inputs/skip-headers/a.h clang-tools-extra/test/clang-tidy/checkers/Inputs/skip-headers/b.h clang-tools-extra/test/clang-tidy/checkers/Inputs/skip-headers/c.h clang-tools-extra/test/clang-tidy/checkers/Inputs/skip-headers/c1.h clang-tools-extra/test/clang-tidy/checkers/Inputs/skip-headers/my_header1.h clang-tools-extra/test/clang-tidy/checkers/Inputs/skip-headers/my_header2.h clang-tools-extra/test/clang-tidy/checkers/Inputs/unused-using-decls.h clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp clang-tools-extra/test/clang-tidy/checkers/abseil-upgrade-duration-conversions.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone-forward-declaration-namespace-header.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone-forward-declaration-namespace.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-interfaces-global-init.cpp clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage-command-line-macros.cpp clang-tools-extra/test/clang-tidy/checkers/google-namespaces.cpp clang-tools-extra/test/clang-tidy/checkers/google-objc-function-naming.m clang-tools-extra/test/clang-tidy/checkers/google-upgrade-googletest-case.cpp clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp clang-tools-extra/test/clang-tidy/checkers/llvm-prefer-register-over-unsigned.cpp clang-tools-extra/test/clang-tidy/checkers/llvmlibc-implementation-in-namespace.cpp clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers.cpp clang-tools-extra/test/clang-tidy/checkers/misc-no-recursion.cpp clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-cxx03.cpp clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-cxx11.cpp clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value-header.cpp clang-tools-extra/test/clang-tidy/checkers/portability-restrict-system-includes-allow.cpp clang-tools-extra/test/clang-tidy/checkers/portability-restrict-system-includes-disallow.cpp clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-chained-conditional-assignment.cpp clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-chained-conditional-return.cpp clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-members.cpp clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp clang-tools-extra/test/clang-tidy/checkers/skip-headers-1.cpp clang-tools-extra/test/clang-tidy/checkers/skip-headers-2.cpp clang-tools-extra/test/clang-tidy/checkers/skip-headers-3.cpp clang-tools-extra/test/clang-tidy/checkers/skip-headers-4.cpp clang-tools-extra/test/clang-tidy/infrastructure/file-filter-symlinks.cpp clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp clang-tools-extra/test/clang-tidy/infrastructure/line-filter.cpp clang/include/clang/ASTMatchers/ASTMatchFinder.h clang/include/clang/Frontend/MultiplexConsumer.h clang/lib/ASTMatchers/ASTMatchFinder.cpp Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp === --- clang/lib/ASTMatchers/ASTMatchFinder.cpp +++ clang/lib/ASTMatchers/ASTMatchFinder.cpp @@ -1202,6 +1202,8 @@ if (!DeclNode) { return true; } + if (Options.Filter && Options.Filter->skipLocation(DeclNode-
[PATCH] D107323: [clang-tidy] Add skip-headers; use skipLocation and setTraversalScope
chh updated this revision to Diff 367331. chh added a comment. fix windows test failure CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107323/new/ https://reviews.llvm.org/D107323 Files: clang-tools-extra/clang-tidy/ClangTidy.cpp clang-tools-extra/clang-tidy/ClangTidyCheck.h clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h clang-tools-extra/clang-tidy/ClangTidyModule.cpp clang-tools-extra/clang-tidy/ClangTidyModule.h clang-tools-extra/clang-tidy/ClangTidyOptions.cpp clang-tools-extra/clang-tidy/ClangTidyOptions.h clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-forward-declaration-namespace/a.h clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-forward-declaration-namespace/b.h clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-pass-by-value/header.h clang-tools-extra/test/clang-tidy/checkers/Inputs/skip-headers/a.h clang-tools-extra/test/clang-tidy/checkers/Inputs/skip-headers/b.h clang-tools-extra/test/clang-tidy/checkers/Inputs/skip-headers/c.h clang-tools-extra/test/clang-tidy/checkers/Inputs/skip-headers/c1.h clang-tools-extra/test/clang-tidy/checkers/Inputs/skip-headers/my_header1.h clang-tools-extra/test/clang-tidy/checkers/Inputs/skip-headers/my_header2.h clang-tools-extra/test/clang-tidy/checkers/Inputs/unused-using-decls.h clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp clang-tools-extra/test/clang-tidy/checkers/abseil-upgrade-duration-conversions.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone-forward-declaration-namespace-header.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone-forward-declaration-namespace.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-interfaces-global-init.cpp clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage-command-line-macros.cpp clang-tools-extra/test/clang-tidy/checkers/google-namespaces.cpp clang-tools-extra/test/clang-tidy/checkers/google-objc-function-naming.m clang-tools-extra/test/clang-tidy/checkers/google-upgrade-googletest-case.cpp clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp clang-tools-extra/test/clang-tidy/checkers/llvm-prefer-register-over-unsigned.cpp clang-tools-extra/test/clang-tidy/checkers/llvmlibc-implementation-in-namespace.cpp clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers.cpp clang-tools-extra/test/clang-tidy/checkers/misc-no-recursion.cpp clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-cxx03.cpp clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-cxx11.cpp clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value-header.cpp clang-tools-extra/test/clang-tidy/checkers/portability-restrict-system-includes-allow.cpp clang-tools-extra/test/clang-tidy/checkers/portability-restrict-system-includes-disallow.cpp clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-chained-conditional-assignment.cpp clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-chained-conditional-return.cpp clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-members.cpp clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp clang-tools-extra/test/clang-tidy/checkers/skip-headers-1.cpp clang-tools-extra/test/clang-tidy/checkers/skip-headers-2.cpp clang-tools-extra/test/clang-tidy/checkers/skip-headers-3.cpp clang-tools-extra/test/clang-tidy/checkers/skip-headers-4.cpp clang-tools-extra/test/clang-tidy/infrastructure/file-filter-symlinks.cpp clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp clang-tools-extra/test/clang-tidy/infrastructure/line-filter.cpp clang/include/clang/ASTMatchers/ASTMatchFinder.h clang/include/clang/Frontend/MultiplexConsumer.h clang/lib/ASTMatchers/ASTMatchFinder.cpp Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp === --- clang/lib/ASTMatchers/ASTMatchFinder.cpp +++ clang/lib/ASTMatchers/ASTMatchFinder.cpp @@ -1202,6 +1202,8 @@ if (!DeclNode) { return true; } + if (Options.Filter && Options.Filter->skipLocation(DeclNode->getLocation())) +return true; bool ScopedTraversal =