[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 06:00:19 PDT 2020


aaron.ballman added inline comments.


================
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
----------------
njames93 wrote:
> aaron.ballman wrote:
> > Should remove commented-out code from the list.
> Once we are happy I will remove it, more  a temporary step.
SGTM!


================
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",
----------------
njames93 wrote:
> aaron.ballman wrote:
> > 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.
> This function should have this signature if default arguments were allowed.
> ```
> extern void* __builtin_assume_aligned(const void* exp, size_t align, size_t misaligned = 0);```
Ah, I see now. Makes sense to keep it, but maybe update the comment as "documented as variadic to support default parameters".

If you've vetted the other builtins below to see that they're not actually variadic, I'm fine leaving them in the list as, btw.


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