[PATCH] D112579: Allow non-variadic functions to be attributed with `__attribute__((format))`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D112579#3630647 <https://reviews.llvm.org/D112579#3630647>, @fcloutier wrote:

> I'm afraid that's also not possible: `D` is a `Decl`, so it doesn't have `getType()`. `Decl` is the tightest-fitting superclass of `BlockDecl`, `FunctionDecl` and `ObjCMethodDecl` (because `BlockDecl` is a direct subclass of it).
>
> One option could be to cast the `Decl` to a `FunctionDecl` and then use `FDecl->isVariadic()`, similarly to how it goes for `BlockDecl` and `ObjCMethodDecl`. I'm not sure that it's equivalent, but if you believe it is and like it better, I can do that.

Ahhhhhh, I had forgotten about `BlockDecl` not being a `ValueDecl`. In that case, I think the code is fine as-is, sorry for the noise!

I think this generally LG; I found a few minor nits in the documentation and some questions on the tests. The test stuff can be handled in a follow-up if you think it's worthwhile.



================
Comment at: clang/include/clang/Basic/AttrDocs.td:3147
+
+As an extension to GCC's behavior, Clang accepts the format attribute on
+non-variadic functions. Clang checks non-variadic format functions for the same
----------------



================
Comment at: clang/include/clang/Basic/AttrDocs.td:3165
+
+Using the format attribute on a non-variadic function emits a GCC compatibility
+diagnostic.
----------------



================
Comment at: clang/test/SemaCXX/attr-format.cpp:76
+  format("bare string");
+  format("%s", 123); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
+  format("%s %s %u %d %i %p\n", "hello", s, 10u, x, y, &do_format);
----------------
This pointed out an interesting test case. What should the behavior be for:
```
format("%p", 0);
```
Because that sure feels like a more reasonable thing for someone to write expecting it to be treated as a null pointer constant.


================
Comment at: clang/test/SemaCXX/attr-format.cpp:77-78
+  format("%s", 123); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
+  format("%s %s %u %d %i %p\n", "hello", s, 10u, x, y, &do_format);
+  format("%s %s %u %d %i %p\n", "hello", s, 10u, x, y, do_format);
+  format("bad format %s"); // expected-warning{{more '%' conversions than data arguments}}
----------------
This likely isn't specific to your changes, but the `%p` in these examples should be warning the user (a function or function pointer is not a pointer to void or a pointer to a character type, so that call is UB).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112579