[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17

Arthur O'Dwyer via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 3 13:28:54 PDT 2018


Quuxplusone added a comment.

>> If we must have an opt-in/out mechanism (which I don't believe we do)
> 
> Yes, we do. Opt-out is pre-existing, and removing it would be an [unacceptable] regression.

Ah, I see now that you weren't the originator of `_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17`; that existed on the left-hand side of this diff already. Okay, I don't object to keeping it around for historical interest.

> Opt-in is an enhancement. Of course, it would be nice to always default it to on, but as it was disscussed with @mclow.lists, this is simply not going to happen. This is the best we'll get.

Is @mclow.lists on record anywhere as saying that, that you could link to? I ask because I haven't seen him commenting on these patches. At the moment, it seems like you're alone in both pushing these patches and simultaneously arguing that any effort is futile. :)

>>   #ifdef _LIBCPP_NODISCARD
>>      // the user has given us their preferred spelling; use it unconditionally
> 
> So you propose to shift the burden of picking which define to use to each and every libc++ user

Not at all!  I merely suggested that rather than having three different macros concerned with the spelling of `nodiscard`, you reduce it to *one* macro: `_LIBCPP_NODISCARD`. There would be a "sensible default" (namely, whatever spelling Does The Right Thing on the detected compiler), and then users who didn't want that default would be free to override it by passing `-D` on the command line. The `#ifdef` I suggested is a clean, concise way of allowing the user's explicit choice to override the libc++-provided "sensible default", no matter whether the user is trying to opt-in on an unsupported compiler (`-D_LIBCPP_NODISCARD='[[foobar::nodiscard]]'`) or opt-out on a supported compiler (`-D_LIBCPP_NODISCARD=''`). It would be a way to eliminate the soup of `ENABLE_` and `DISABLE_` macros.

Of course the benefit of this idea decreases if we must preserve `_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17` for historical reasons anyway.


Repository:
  rCXX libc++

https://reviews.llvm.org/D45179





More information about the cfe-commits mailing list