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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 3 04:54:00 PDT 2020


aaron.ballman added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:39
+    "__builtin_classify_type",
+    // "__builtin_va_start",
+    // "__builtin_stdarg_start",
----------------
I think we may want to keep this one as it's the builtin for the `va_start` macro, which is not variadic.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:40
+    // "__builtin_va_start",
+    // "__builtin_stdarg_start",
+    "__builtin_assume_aligned", // Documented as variadic to support overloading
----------------
Should remove commented-out code from the list.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:41
+    // "__builtin_stdarg_start",
+    "__builtin_assume_aligned", // Documented as variadic to support overloading
+    // "__builtin_fprintf",
----------------
If it's documented as being variadic, I think we want to remove it from the list so we warn on its use, right?

>From here on down look like things that are not part of any standard, also, so we may want to remove them based on that, too.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:49
+    // "__builtin___printf_chk",
+    "__builtin_prefetch",      // Documented as variadic to support overloading
+    "__builtin_shufflevector", // Documented as variadic but with a defined
----------------
Same here.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:63
+    // "__builtin_os_log_format",
+    // "__builtin_ms_va_start",
+    // clang-format on
----------------
Except for this one -- I think we may want to keep this one because it's the MS version of `va_start`, which is not variadic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80887





More information about the cfe-commits mailing list