[clang] [clang] Generate note on declaration for nodiscard-related attributes (PR #112289)

[Yihe Li via cfe-commits]

Mick235711 wrote:

> I don’t really see that happening for nodiscard tho—making only some overloads nodiscard instead of none or all of them just seems like a really weird thing to do personally...

I think there is still some merits for these kind of overload sets. For instance, API like [`basic_spanstream::span()`](https://en.cppreference.com/w/cpp/io/basic_spanstream/span), where the same name is used for both setter and getter, is pretty common out in the wild:
```cpp
std::span<CharT> span() const noexcept;
void span( std::span<CharT> s ) noexcept;
```
In these kind of cases the first overload can be marked [[nodiscard]], which is probably reasonable.

> As I said, I feel like that might be by design, though.

> at least for deprecated, you could maybe argue that you might want to know what part of the codebase marked it as deprecated in case there are multiple declarations

Indeed, I'm currently torn about this. On the one hand, the same argument can be applied to nodiscard, where you might want to know what part of the codebase marked it as nodiscard in case of multiple declarations. On the other hand, the motivation is definitely weaker here.

> The one use case I could see is that the nodiscard is on the TYPE instead of the function itself (which, as you said before, is goofy for an overload set with this only partially applied). But I don't really see value besides "this should not be discarded", and the actual location of the attribute isn't particularly valuable. What IS important is the 'reason', which we already have.

I can suggest one motivation here, which potentially applies to [[nodiscard]] on types. In the standard, it is allowed to mark only some specializations of a class template as nodiscard:
```cpp
template<typename T> struct S {};
template<> struct [[nodiscard]] S<int> {};

template<typename T>
S<T> getS(const T&) { return {}; }

int main()
{
    getS(2.0); // no warn
    getS(2); // warns
}
```
In this case, the current warning is not especially good, the only thing mentioned is: ([Compiler Explorer](https://godbolt.org/z/4cWqPfo4T))
```cpp
<source>:10:5: warning: ignoring return value of function declared with 'nodiscard' attribute [-Wunused-result]
   10 |     getS(2);
      |     ^~~~ ~
1 warning generated.
```
Nowhere is the return type `S` actually mentioned, let alone the specific specialization. Under this PR, this will instead generate:
```cpp
test-2.cpp:10:5: warning: ignoring return value of function declared with 'nodiscard' attribute [-Wunused-result]
   10 |     getS(2);
      |     ^~~~ ~
test-2.cpp:2:21: note: 'S<int>' has been explicitly marked nodiscard here
    2 | template<> struct [[nodiscard]] S<int> {};
      |                     ^
1 warning generated.
```
Here the specialization is named, which is potentially helpful. Although I'll admit this is a bit of a contrived example.

The noise argument does makes sense though, so I'm currently undecided about this too.

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