[PATCH] D60872: Add new warning knob for unknown attribute namespaces

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Hmm. So there are a few different situations where we might meet an unknown attribute (I'm sure I missed some):

1. The attribute had a typo in it (eg: [[clagn::fallthrough]]).
2. The attribute has semantic effects (ignoring it is incorrect and will -- at best -- not compile or -- at worst -- generate a subtly broken program, eg: [[gnu::aligned]] or [[gnu::target]]).
3. The attribute has important effects (ignoring it is probably correct but suboptimal in some way, eg: [[no_unique_address]], [[gnu::packed]], ...).
4. The attribute has unimportant effects (is entirely safely ignorable) or is only intended to affect the behavior of a different tool (eg: gsl, static analyzer, clang-tidy, etc).

I think the ideal would be to warn on unknown attributes in cases 1 and 2, optionally warn on unknown attributes in case 3, and to not warn in case 4. Without user hints, we can't tell which is which in general.

"Do not warn on unknown namespaces" gives us some part of not warning on case 4. However, it also turns off warnings in cases 1-3 (eg, we won't warn on `[[gcc::noinline]]` as a typo for `[[gnu::noinline]]`), and doesn't turn off warnings for case-4 attributes in, say, namespace `gnu`, so it's somewhat imperfect. I think it's also going to be surprising that the clang release that starts parsing a [[gsl::*]] attribute also starts warning on other ("unknown") [[gsl::*]] attributes for people in this new mode.

It seems to me that we can achieve the ideal (don't warn on safely-ignorable unknown attributes, but warn on all other unknown attributes) if we ask the user to give us a list of safely-ignorable attributes and attribute namespaces that they intend to use. (That even lets us do typo-correction for attributes we don't support.) In the cfe-dev thread, there was mention that there are hundreds of attributes supported by clang, and so hundreds of attributes would need to be listed, but that doesn't follow: you only need to list the attributes that you actually intend to use. That should be a much smaller list (especially once modules adoption picks up, and you don't need to list those attributes used by your dependencies).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60872/new/

https://reviews.llvm.org/D60872