-Wformat-nonliteral was turned on in https://reviews.llvm.org/D112927,
however we forgot to apply some format attributes in Linux specific
code paths, which led to warnings when building on Linux. This patch
addresses that oversight.
Details
- Reviewers
uabelho Mordante - Group Reviewers
Restricted Project - Commits
- rG7dc9a03cfd78: [libc++] Add missing __format__ attributes
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Thanks for looking into this! I missed those because I only built the library on Darwin, and CI passed since those are only warnings 🤦🏼♂️.
libcxx/include/__bsd_locale_fallbacks.h | ||
---|---|---|
112 | I believe we should instead move those to _LIBCPP_FORMAT_PRINTF. The usage is documented here: https://clang.llvm.org/docs/AttributeReference.html#format. |
libcxx/include/__bsd_locale_fallbacks.h | ||
---|---|---|
112 | Would you feel like doing it if it should be fixed in some other way that just disabling the warning? I really don't know this stuff, I just saw one of our downstream bots signalling there were a bunch of new warnings and saw you disabled warnings this way in your original patch so did the same thing here. |
libcxx/include/__bsd_locale_fallbacks.h | ||
---|---|---|
112 | Yup, no worries. Thanks for bringing that to our attention! |
LGTM modulo one remark.
libcxx/include/__config | ||
---|---|---|
1355 | I like the comment, but let's expand it a bit. This change looks very familiar. I'm quite sure something similar has been up for review before, but seems it never landed. |
libcxx/include/__bsd_locale_fallbacks.h | ||
---|---|---|
112 | Great! Thank you! |
Address review comments and avoid adding _LIBCPP_HIDE_FROM_ABI, which breaks the GCC build.
We can address that separately.
Will ship once CI passes.
libcxx/include/__config | ||
---|---|---|
1355 | Yeah, now that you mention it this does seem to ring a bell for me too. But in any case, it hasn't landed yet so I guess we'll eventually stumble upon the change while clearing the review queue. |
I believe we should instead move those to _LIBCPP_FORMAT_PRINTF. The usage is documented here: https://clang.llvm.org/docs/AttributeReference.html#format.