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