[PATCH] D27165: Add format_dynamic_key_arg attribute to improve "-Wformat" warnings for functions that load the formatting string dynamically based on a key value

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: include/clang/Basic/Attr.td:862
+def FormatDynamicKeyArg : InheritableAttr {
+  let Spellings = [GCC<"format_dynamic_key_arg">];
+  let Args = [IntArgument<"FormatIdx">];
----------------
Does GCC support this attribute as well? If not, this should use the GNU spelling instead of the GCC one. Also, should this have a C++11 spelling in the clang namespace?

The name doesn't really convey much about what the attribute is doing, mostly because it doesn't seem obvious what "key" means.

It seems that the crux of what this attribute says is that the attributed function accepts a string argument of a format specifier and returns the same format specifier. Perhaps `returns_format_specifier` is a better name?


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5632
+  case AttributeList::AT_FormatDynamicKeyArg:
+    handleFormatArgAttr(S, D, Attr, /*IsDynamicKey=*/true);
+    break;
----------------
We ask that all of these `handle*Attr()` functions have the same signature, so adding extra parameters isn't something we do. At some point, we want to generate more boilerplate code from the attribute definition files, and so having different parameter lists makes that much harder to accomplish. A better approach would be to make a function called `handleFormatDynamicKeyArg()` that calls a helper function with the extra argument, and modify `handleFormatArgAttr()` to call that helper function as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D27165