carlosgalvezp created this revision.
carlosgalvezp added reviewers: aaron.ballman, whisperity, salman-javed-nz.
carlosgalvezp added a project: clang-tools-extra.
Herald added subscribers: rnkovacs, kbarton, nemanjai.
carlosgalvezp requested review of this revision.
Herald added a subscriber: cfe-commits.
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.
Update the cppcoreguidelines check to use a PP callback
in order to find the problematic macro at macro expansion
time instead. The old AST matcher is kept for Windows
compatibility.
Add unit test that ensures warnings from macros are disabled
when not using the -system-headers flag. Document the change
in the Release Notes.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D116378
Files:
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
Index: clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp
+++ 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 @@
// 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
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/file-filter/system/system-header.h
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/Inputs/file-filter/system/system-header.h
+++ 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))
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,10 +67,13 @@
Improvements to clang-tidy
--------------------------
-- Added support for globbing in `NOLINT*` expressions, to simplify suppressing
+- 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.
-- Added support for `NOLINTBEGIN` ... `NOLINTEND` comments to suppress
+- Added support for ``NOLINTBEGIN`` ... ``NOLINTEND`` comments to suppress
Clang-Tidy warnings over multiple lines.
New checks
@@ -148,7 +151,7 @@
- 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
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h
@@ -28,6 +28,8 @@
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;
};
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
@@ -11,47 +11,68 @@
#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;
namespace clang {
namespace tidy {
namespace cppcoreguidelines {
+namespace {
+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(),
+ "do not use va_arg to define c-style vararg functions; "
+ "use variadic templates instead");
+ }
+ }
+
+private:
+ ProTypeVarargCheck *Check;
+};
+} // namespace
const internal::VariadicDynCastAllOfMatcher<Stmt, VAArgExpr> VAArgExpr;
static constexpr StringRef AllowedVariadics[] = {
// clang-format off
- "__builtin_isgreater",
- "__builtin_isgreaterequal",
+ "__builtin_isgreater",
+ "__builtin_isgreaterequal",
"__builtin_isless",
- "__builtin_islessequal",
- "__builtin_islessgreater",
+ "__builtin_islessequal",
+ "__builtin_islessgreater",
"__builtin_isunordered",
- "__builtin_fpclassify",
- "__builtin_isfinite",
+ "__builtin_fpclassify",
+ "__builtin_isfinite",
"__builtin_isinf",
- "__builtin_isinf_sign",
- "__builtin_isnan",
+ "__builtin_isinf_sign",
+ "__builtin_isnan",
"__builtin_isnormal",
- "__builtin_signbit",
- "__builtin_constant_p",
+ "__builtin_signbit",
+ "__builtin_constant_p",
"__builtin_classify_type",
"__builtin_va_start",
- "__builtin_assume_aligned", // Documented as variadic to support default
+ "__builtin_assume_aligned", // Documented as variadic to support default
// parameters.
"__builtin_prefetch", // Documented as variadic to support default
// parameters.
"__builtin_shufflevector", // Documented as variadic but with a defined
// number of args based on vector size.
- "__builtin_convertvector",
+ "__builtin_convertvector",
"__builtin_call_with_static_chain",
- "__builtin_annotation",
- "__builtin_add_overflow",
+ "__builtin_annotation",
+ "__builtin_add_overflow",
"__builtin_sub_overflow",
- "__builtin_mul_overflow",
+ "__builtin_mul_overflow",
"__builtin_preserve_access_index",
- "__builtin_nontemporal_store",
+ "__builtin_nontemporal_store",
"__builtin_nontemporal_load",
"__builtin_ms_va_start",
// clang-format on
@@ -125,6 +146,12 @@
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)
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -719,7 +719,7 @@
}
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
