[PATCH] D149514: Check if First argument in _builtin_assume_aligned_ is of pointer type

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added inline comments.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-vararg.cpp:55
 void ignoredBuiltinsTest() {
-  (void)__builtin_assume_aligned(0, 8);
   (void)__builtin_constant_p(0);
----------------
The test should not be removed, it is here for a reason.
In fact, passing zero to a pointer parameter is valid in C and is accepted in C++ mode with a diagnostic (-Wzero-as-null-pointer-constant, disabled by default).



================
Comment at: clang/lib/Sema/SemaChecking.cpp:7984
       return true;
+    QualType firstArgType = FirstArgResult.get()->getType();
+
----------------
Variables should have CamelCase names.



================
Comment at: clang/lib/Sema/SemaChecking.cpp:7986
+
+    if (!firstArgType->isAnyPointerType()) {
+      QualType expectedType = Context.getPointerType(firstArgType);
----------------
Instead of directly checking for the pointer type, it is better to call `checkBuiltinArgument`, which will handle many corner cases.
See the uses of this function in this file.



================
Comment at: clang/test/Sema/builtin-assume-aligned.c:75
+int test14(int *a, int b) {
+  a = (int *)__builtin_assume_aligned(b, 32); // expected-error {{passing 'int' to parameter of incompatible type 'int *'}}
+  return a[0];
----------------
The expected type is not `int *`, it is `cost void *` (according to the definition of the builtin in Builtins.def).



================
Comment at: clang/test/Sema/builtin-assume-aligned.c:77
+  return a[0];
+}
+
----------------
Please add a test that passes an array to the builtin. It should pass.



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

https://reviews.llvm.org/D149514