[PATCH] D129835: [clang] adds a discardable attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a subscriber: ldionne.
cjdb added a comment.

In D129835#3655487 <https://reviews.llvm.org/D129835#3655487>, @aaron.ballman wrote:

>> When we have a function that can have its value discarded, we can use [[clang::discardable]] to indicate that the `[[nodiscard]]` attribute should be ignored.
>
> Alternatively, you can scope the pragma and declarations appropriately and then we don't need to add a new attribute. Is there a reason that wouldn't work for you?

A key problem here (IMO) is that this requires manually pushing and popping the pragma as it is required. With `[[discardable]]` one just needs to push/pop at the extremes of a file (and if a future version of module maps supports global pragmas for a module, there too, but that's a discussion that requires a design doc).

It's also good for documenting "hey you can discard this thing in my sea of nodiscard interfaces" on the interface itself. That coupling is also good for correctness because it means something can't accidentally slip in/out of the pragma's scope.

> (This attribute would be interesting to consider if we added a feature flag like `-fdiagnose-discarded-results` where the default is as-if everything was declared `nodiscard` and then you could mark the APIs with `[[clang::discardable]]` for the few whose results don't matter. However, that's also a much bigger feature because system headers are a challenge.)

The primary reason for me wanting to add this attribute comes from my libc++ days, where I wanted to mark whatever made sense as nodiscard. @ldionne was in favour in principle, but against in practice; noting that nodiscard would clutter the interface too much, and it'd be better if the situation were reversed (which is what this patch does). I think adding this is still a good step for getting us to a world where `-fdiagnose-discarded-results` can be a real consideration, especially if libc++ and llvm-libc adopt it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129835