Author: Carlos Galvez Date: 2022-01-06T20:27:28Z New Revision: 670de10f9deaa83f4d1db6e793c74cbfd18c65c1
URL: https://github.com/llvm/llvm-project/commit/670de10f9deaa83f4d1db6e793c74cbfd18c65c1 DIFF: https://github.com/llvm/llvm-project/commit/670de10f9deaa83f4d1db6e793c74cbfd18c65c1.diff LOG: Disable clang-tidy warnings from system macros Currently, it's inconsistent that warnings are disabled if they come from system headers, unless they come from macros. Typically a user cannot act upon these warnings coming from system macros, so clang-tidy should ignore them unless the user specifically requests warnings from system headers via the corresponding configuration. This change broke the ProTypeVarargCheck check, because it was checking for the usage of va_arg indirectly, expanding it (it's a system macro) to detect the usage of __builtin_va_arg. The check has been fixed by checking directly what the rule is about: "do not use va_arg", by adding a PP callback that checks if any macro with name "va_arg" is expanded. The old AST matcher is still kept for compatibility with Windows. Add unit test that ensures warnings from macros are disabled when not using the -system-headers flag. Document the change in the Release Notes. Differential Revision: https://reviews.llvm.org/D116378 Added: Modified: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/infrastructure/Inputs/file-filter/system/system-header.h clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp index 6c5054fdca28e..a76fe16effb6a 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -719,7 +719,7 @@ void ClangTidyDiagnosticConsumer::checkFilters(SourceLocation Location, } if (!*Context.getOptions().SystemHeaders && - Sources.isInSystemHeader(Location)) + (Sources.isInSystemHeader(Location) || Sources.isInSystemMacro(Location))) return; // FIXME: We start with a conservative approach here, but the actual type of diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp index b26cd59fd672a..83924f977d1a6 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp @@ -11,6 +11,9 @@ #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Basic/TargetInfo.h" +#include "clang/Lex/PPCallbacks.h" +#include "clang/Lex/Preprocessor.h" +#include "clang/Lex/Token.h" using namespace clang::ast_matchers; @@ -57,6 +60,10 @@ static constexpr StringRef AllowedVariadics[] = { // clang-format on }; +static constexpr StringRef VaArgWarningMessage = + "do not use va_arg to define c-style vararg functions; " + "use variadic templates instead"; + namespace { AST_MATCHER(QualType, isVAList) { ASTContext &Context = Finder->getASTContext(); @@ -106,6 +113,21 @@ AST_MATCHER_P(AdjustedType, hasOriginalType, ast_matchers::internal::Matcher<QualType>, InnerType) { return InnerType.matches(Node.getOriginalType(), Finder, Builder); } + +class VaArgPPCallbacks : public PPCallbacks { +public: + VaArgPPCallbacks(ProTypeVarargCheck *Check) : Check(Check) {} + + void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD, + SourceRange Range, const MacroArgs *Args) override { + if (MacroNameTok.getIdentifierInfo()->getName() == "va_arg") { + Check->diag(MacroNameTok.getLocation(), VaArgWarningMessage); + } + } + +private: + ProTypeVarargCheck *Check; +}; } // namespace void ProTypeVarargCheck::registerMatchers(MatchFinder *Finder) { @@ -125,6 +147,12 @@ void ProTypeVarargCheck::registerMatchers(MatchFinder *Finder) { this); } +void ProTypeVarargCheck::registerPPCallbacks(const SourceManager &SM, + Preprocessor *PP, + Preprocessor *ModuleExpanderPP) { + PP->addPPCallbacks(std::make_unique<VaArgPPCallbacks>(this)); +} + static bool hasSingleVariadicArgumentWithValue(const CallExpr *C, uint64_t I) { const auto *FDecl = dyn_cast<FunctionDecl>(C->getCalleeDecl()); if (!FDecl) @@ -153,9 +181,7 @@ void ProTypeVarargCheck::check(const MatchFinder::MatchResult &Result) { } if (const auto *Matched = Result.Nodes.getNodeAs<Expr>("va_use")) { - diag(Matched->getExprLoc(), - "do not use va_arg to define c-style vararg functions; " - "use variadic templates instead"); + diag(Matched->getExprLoc(), VaArgWarningMessage); } if (const auto *Matched = Result.Nodes.getNodeAs<VarDecl>("va_list")) { diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h index 06de06e3e8361..08b879abf130a 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h @@ -28,6 +28,8 @@ class ProTypeVarargCheck : public ClangTidyCheck { return LangOpts.CPlusPlus; } void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; }; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 5d330d1d2e3cf..03a52fa7c7aec 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -67,6 +67,9 @@ The improvements are... Improvements to clang-tidy -------------------------- +- Ignore warnings from macros defined in system headers, if not using the + `-system-headers` flag. + - Added support for globbing in `NOLINT*` expressions, to simplify suppressing multiple warnings in the same line. @@ -151,7 +154,7 @@ Changes in existing checks - Fixed a false positive in :doc:`fuchsia-trailing-return <clang-tidy/checks/fuchsia-trailing-return>` for C++17 deduction guides. - + - Fixed a false positive in :doc:`bugprone-throw-keyword-missing <clang-tidy/checks/bugprone-throw-keyword-missing>` when creating an exception object using placement new diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/file-filter/system/system-header.h b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/file-filter/system/system-header.h index 98482c4277dda..6e338c85f394e 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/file-filter/system/system-header.h +++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/file-filter/system/system-header.h @@ -1 +1,3 @@ class A0 { A0(int); }; + +#define TO_FLOAT_PTR(x) ((float *)(x)) diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp index 9ee5cad979f6a..a7498723de2bf 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp @@ -9,6 +9,8 @@ // file-filter\header*.h due to code order between '/' and '\\'. // RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers %s -- -I %S/Inputs/file-filter/system/.. -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK4 %s // RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers -quiet %s -- -I %S/Inputs/file-filter/system/.. -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK4-QUIET %s +// RUN: clang-tidy -checks='-*,cppcoreguidelines-pro-type-cstyle-cast' -header-filter='.*' -system-headers %s -- -I %S/Inputs/file-filter/system/.. -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK5 %s +// RUN: clang-tidy -checks='-*,cppcoreguidelines-pro-type-cstyle-cast' -header-filter='.*' %s -- -I %S/Inputs/file-filter/system/.. -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK5-NO-SYSTEM-HEADERS %s #include "header1.h" // CHECK-NOT: warning: @@ -71,3 +73,8 @@ class A { A(int); }; // CHECK4-NOT: Suppressed {{.*}} warnings // CHECK4-NOT: Use -header-filter=.* {{.*}} // CHECK4-QUIET-NOT: Suppressed + +int x = 123; +auto x_ptr = TO_FLOAT_PTR(&x); +// CHECK5: :[[@LINE-1]]:14: warning: do not use C-style cast to convert between unrelated types +// CHECK5-NO-SYSTEM-HEADERS-NOT: :[[@LINE-2]]:14: warning: do not use C-style cast to convert between unrelated types _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
