[clang-tools-extra] e21c3f2 - [clang-tidy] ignore builtin varargs from pro-type-vararg-check

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-06-04T17:58:23+01:00
New Revision: e21c3f223a3537df0d9d02fce5038b3d7b98ab52

URL: https://github.com/llvm/llvm-project/commit/e21c3f223a3537df0d9d02fce5038b3d7b98ab52
DIFF: https://github.com/llvm/llvm-project/commit/e21c3f223a3537df0d9d02fce5038b3d7b98ab52.diff

LOG: [clang-tidy] ignore builtin varargs from pro-type-vararg-check

Disables the check from warning on some built in vararg functions, Address [[ https://bugs.llvm.org/show_bug.cgi?id=45860 | Clang-tidy should not consider __builtin_constant_p a variadic function. ]]

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D80887

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
    clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
index 4509c934ddc5..f6aeb8321337 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
@@ -9,6 +9,7 @@
 #include "ProTypeVarargCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 
 using namespace clang::ast_matchers;
 
@@ -18,11 +19,72 @@ namespace cppcoreguidelines {
 
 const internal::VariadicDynCastAllOfMatcher<Stmt, VAArgExpr> vAArgExpr;
 
+static constexpr StringRef AllowedVariadics[] = {
+    // clang-format off
+    "__builtin_isgreater", 
+    "__builtin_isgreaterequal", 
+    "__builtin_isless",
+    "__builtin_islessequal", 
+    "__builtin_islessgreater", 
+    "__builtin_isunordered",
+    "__builtin_fpclassify", 
+    "__builtin_isfinite", 
+    "__builtin_isinf",
+    "__builtin_isinf_sign", 
+    "__builtin_isnan", 
+    "__builtin_isnormal",
+    "__builtin_signbit", 
+    "__builtin_constant_p", 
+    "__builtin_classify_type",
+    "__builtin_va_start",
+    "__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_call_with_static_chain",
+    "__builtin_annotation", 
+    "__builtin_add_overflow", 
+    "__builtin_sub_overflow",
+    "__builtin_mul_overflow", 
+    "__builtin_preserve_access_index",
+    "__builtin_nontemporal_store", 
+    "__builtin_nontemporal_load",
+    "__builtin_ms_va_start",
+    // clang-format on
+};
+
+namespace {
+AST_MATCHER(QualType, isVAList) {
+  ASTContext &Context = Finder->getASTContext();
+  QualType Desugar = Node.getDesugaredType(Context);
+  return Context.getBuiltinVaListType().getDesugaredType(Context) == Desugar ||
+         Context.getBuiltinMSVaListType().getDesugaredType(Context) == Desugar;
+}
+
+AST_MATCHER_P(AdjustedType, hasOriginalType,
+              ast_matchers::internal::Matcher<QualType>, InnerType) {
+  return InnerType.matches(Node.getOriginalType(), Finder, Builder);
+}
+} // namespace
+
 void ProTypeVarargCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(vAArgExpr().bind("va_use"), this);
 
   Finder->addMatcher(
-      callExpr(callee(functionDecl(isVariadic()))).bind("callvararg"), this);
+      callExpr(callee(functionDecl(isVariadic(),
+                                   unless(hasAnyName(AllowedVariadics)))))
+          .bind("callvararg"),
+      this);
+
+  Finder->addMatcher(
+      varDecl(unless(parmVarDecl()),
+              hasType(qualType(
+                  anyOf(isVAList(), decayedType(hasOriginalType(isVAList()))))))
+          .bind("va_list"),
+      this);
 }
 
 static bool hasSingleVariadicArgumentWithValue(const CallExpr *C, uint64_t I) {
@@ -54,7 +116,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_start/va_arg to define c-style vararg functions; "
+         "do not use va_arg to define c-style vararg functions; "
          "use variadic templates instead");
   }
 

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg.cpp
index 021322aacb62..1e3b5ee036ca 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg.cpp
@@ -39,13 +39,22 @@ void CallFooIfAvailable(T& t) {
 #include <stdarg.h>
 void my_printf(const char* format, ...) {
   va_list ap;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare variables of type va_list; use variadic templates instead
   va_start(ap, format);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not call c-style vararg functions
   va_list n;
-  va_copy(n, ap); // Don't warn, va_copy is anyway useless without va_start
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare variables of type va_list; use variadic templates instead
+  va_copy(n, ap);
   int i = va_arg(ap, int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use va_start/va_arg to define c-style vararg functions; use variadic templates instead
-  va_end(ap); // Don't warn, va_end is anyway useless without va_start
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use va_arg to define c-style vararg functions; use variadic templates instead
+  va_end(ap);
 }
 
 int my_vprintf(const char* format, va_list arg ); // OK to declare function taking va_list
+
+void ignoredBuiltinsTest() {
+  (void)__builtin_assume_aligned(0, 8);
+  (void)__builtin_constant_p(0);
+  (void)__builtin_fpclassify(0, 0, 0, 0, 0, 0.f);
+  (void)__builtin_isinf_sign(0.f);
+  (void)__builtin_prefetch(nullptr);
+}