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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
