arphaman 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">];
----------------
aaron.ballman wrote:
> 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?
> Does GCC support this attribute as well? If not, this should use the GNU
> spelling instead of the GCC one.
No, it doesn't. Thanks for pointing that out, I fixed it.
> Also, should this have a C++11 spelling in the clang namespace?
I don't know, is that required for new attributes? I don't need the C++11
spelling personally, and I don't know if there is anyone else who's interested
in this attribute.
> 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?
You're right, the name should convey the intended meaning better. I switched to
`loads_format_specifier_value_using_key`, how does that sound?
I think it sounds better than just `returns_format_specifier` because I would
like to see this attribute used only for a particular kind of function. This
kind of function should load the value at runtime using the key from a
platform-specific file/database that might also be accessible at compile-time.
It shouldn't be used for functions that might just modify the given key, which,
IMO, `returns_format_specifier` can imply.
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
