Hi folks,
I was out for a bit, but now I’m back. I see there’s been a lot of discussion about [[nodiscard]] lately (here and on Discord), and I think we should try to settle that story once and for all. My opinion has two aspects to it:
First, I think that we should apply [[nodiscard]] conservatively, only in the places where it is mandated by the Standard, or where we know almost for sure that it is a vexing bug hidden (like std::vector::empty or ranges::empty). Something like std::vector::size, for example, is not a good candidate for getting [[nodiscard]]. Yes, it doesn’t make much sense to call .size() if you discard the result. However, it is not something that we have evidence of it causing bugs, unlike for empty or things like lock_guard’s constructor. Catching vexing cases like .empty() and lock_guard is the primary reason for adding the attribute - policing pretty much all functions that return non-void isn’t.
If we were designing the language from scratch, I’d probably argue that all non-void functions should be [[nodiscard]] implicitly, and then we’d have an attribute to allow discarding the result instead. I think that would be far more ergonomic. But that’s not the language we have, and I think we should try to work with it instead of starting to annotate almost every single function we write with an attribute. Annotating everything has a cost in readability, doesn’t provide obvious value (I’m not convinced that annotating something like .size() will help catch any bug, ever), and will almost certainly cause issues for people trying to update to the new version of the library, in case they use some [[nodiscard]] functions in dubious but valid places. That brings us to the second point about extensions vs mandated [[nodiscard]].
I believe that if we stick with the guideline above of only adding [[nodiscard]] to places that are truly vexing (and so we should basically consider it a bug that the Standard doesn’t mandate [[nodiscard]] on those), the debate of whether to enable [[nodiscard]] extensions by default or not becomes pretty much moot. Indeed, if we only add [[nodiscard]] conservatively to places that are really catching bugs, we shouldn’t trigger any false positives in user code. Hence, nobody should notice that we added these extensions, and if they do, then they’d be thankful that we’ve done so because we’d have caught a bug. For that reason, I believe that it’s fine to just use [[nodiscard]] unconditionally (not guarded behind some sort of extension macro) if we stay conservative about where we use it. Furthermore, under that guideline, I think we should consider writing a paper to make it part of the Standard every time we add [[nodiscard]] as an extension.
So, my preference would be to review the current uses of [[nodiscard]] in the library (most of which have been added recently) and remove those that do not abide to the guideline above. Similarly, we could turn uses of _LIBCPP_NODISCARD_EXT into _LIBCPP_NODISCARD and update the documentation accordingly (alongside with a release note, but we should expect that change to never break anyone if we’re being conservative).
Louis