carlosgalvezp updated this revision to Diff 397958.
carlosgalvezp added a comment.

- Refactor duplicated warning message into a common variable.
- Remove double backticks change, TBH I get conflicting review comments about 
this and can't find any docs supporting either, so I'll just keep it consistent.
- Move extra class to the existing anonymous namespace instead of creating a 
new one.


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

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,6 +67,9 @@
 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 @@
 
 - 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,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 @@
     // 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_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 @@
       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 @@
   }
 
   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")) {
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

Reply via email to