[libcxx-commits] [libcxx] [libc++] Diagnose unused variables of container types (PR #203084)

[Nikolas Klauser via libcxx-commits]

philnik777 wrote:

> > There are lots of diagnostics which print where some attribute was applied, and macros directly make these diagnostics worse, since you have a list of macro expansions in that case.
> 
> Can you be more specific which warnings do that? It would be helpful to attach a compiler explorer link to demonstrate such an unwanted diagnostics. In my code base, macros wrapped attributes are very common so it's definitely within my duties to seek improvement on those.

`[[deprecated]]` is such an example: https://godbolt.org/z/rE6Eh4cTf. I can't tell you which others exactly produce such diagnostics, but I'm pretty sure there were.

> > Notably, there is also significant precedence of not having a macro, like [[nodiscard]] (with > 3k instances).
> 
> `[[nodiscard]]` is a standard attribute and I don't think it's a good one to compare to. This PR is relying on a vendor extension.

Then let me list some vendor extensions we use:
- type traits builtins
- `if constexpr` pre-C++17
- `explicit(bool)` pre-C++20
- various attributes with underscored names, e.g. `[[__nodiscard__]]`.
- builtin functions
- `inline` variables pre-C++17
- attributes on namespaces pre-C++17
- structured bindings pre-C++17
- lots of C++11 extensions in C++03

I think you get the point.

> > What would you like to see in that description?
> 
> Whether you write it or not, GitHub PR UI would always use some screen real estate to show the description box and that's often the first thing a reviewer would read. It should be rather uncontroversial that any description is better than `No description provided.`

I actually disagree. If your change is simple enough, adding text doesn't provide any value. You've just spent time writing something nobody will ever care about. The length of the description should scale with the complexity of the PR.

> For this particular PR, it would be helpful to state motivations

What sort of motivation do you expect when the first words in the title are "Diagnose unused variables"? I want to diagnose unused variables. Does that need to be re-stated?

> show case an example diagnostic message

I'm actively against diagnostic messages in descriptions. I've never seen a case where I cared what exactly the message looks like; IMO they're just noise.

> and provide a list of container types (or iterator wrappers therein) for reviewer reference.


https://github.com/llvm/llvm-project/pull/203084