erichkeane added a comment. In D129835#4288240 <https://reviews.llvm.org/D129835#4288240>, @aaron.ballman wrote:
> In D129835#4287866 <https://reviews.llvm.org/D129835#4287866>, @cjdb wrote: > >>>> 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). >>> >>> I understood that, I just don't think that's a good thing. This is >>> basically an attribute that says "I know we said we wanted everything here >>> to be nodiscard, but JUST KIDDING not this one!" which is not a very clean >>> approach to writing headers. >> >> @aaron.ballman Ugh, I've finally come up with a good use-case for >> `[[discardable]]`. >> >> class [[nodiscard]] iterator { >> public: >> // usual iterator stuff >> >> [[clang::discardable]] iterator operator++(int); >> [[clang::discardable]] iterator operator--(int); >> }; >> >> I hate it, but the alternative is to decorate everything else with >> `[[nodiscard]]` just to facilitate these two operators. It's a compelling >> use-case, but I'm not sure if it's compelling enough on its own. WDYT? >> (I personally think that those two should be nodiscard, but `i++` is >> pervasive enough that it might never be practical to correct.) > > I'm still not super motivated because I think this encourages a confused > design, because marking a *type* as `nodiscard` is making a promise about the > type as a whole. That asserts there's *never* a valid time to ignore a value > of the type. If you have times when that type is reasonable to ignore, then > the type isn't actually non-discardable, and you should mark the methods > returning the type. (IOW, "nodiscard" is part of the contract for the type in > some limited ways even if it's not reflected by the type system.) > > What's more, I think `iterator` is a case where it'd be a mistake to mark the > type `nodiscard`, because there are plenty times when it's valid to ignore > the type (consider many of the functions in `<algorithm>`, like > `std::copy()`). I think it's more appropriate to mark the functions returning > an iterator rather than the type itself. I'm a little more sympathetic to the use than Aaron here, but I'm still at 'unmotivated' here. As he said, `iterator` I think is a poor candidate for `nodiscard`, as the iterator type itself is not the 'costly to ignore' part here, just the `begin`/`end` methods (and perhaps some of algorithms?). I can see some logic to having this in very limited situations, but I just don't see a motivating use case yet. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129835/new/ https://reviews.llvm.org/D129835 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
