[libc++] Add _LIBCPP_ENABLE_NODISCARD and _LIBCPP_NODISCARD_EXT to allow pre-C++2a [[nodiscard]]

[lebedev.ri]

Summary

The [[nodiscard]] attribute is intended to help users find bugs where
function return values are ignored when they shouldn't be. After C++17 the
C++ standard has started to declared such library functions as [[nodiscard]].
However, this application is limited and applies only to dialects after C++17.
Users who want help diagnosing misuses of STL functions may desire a more
liberal application of [[nodiscard]].

For this reason libc++ provides an extension that does just that! The
extension must be enabled by defining _LIBCPP_ENABLE_NODISCARD. The extended
applications of [[nodiscard]] takes two forms:

1. Backporting [[nodiscard]] to entities declared as such by the standard in newer dialects, but not in the present one.

2. Extended applications of [[nodiscard]], at the libraries discretion, applied to entities never declared as such by the standard.

Users may also opt-out of additional applications [[nodiscard]] using
additional macros.

Applications of the first form, which backport [[nodiscard]] from a newer
dialect may be disabled using macros specific to the dialect it was added. For
example _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17.

Applications of the second form, which are pure extensions, may be disabled
by defining _LIBCPP_DISABLE_NODISCARD_EXT.

This patch was originally by Roman Lebedev.

[lebedev.ri]

Actually use [[gnu::nodiscard]] when meant to

[lebedev.ri]

It's [[*::warn_unused_result]], not [[*::nodiscard]]. Failed to copy-paste from godbolt.

Also, @thakis notes:

[22:39:11] <thakis_> LebedevRI: also, less combinatorial explosion if you do `#if __has_cpp_attribute(nodiscard) #define _LIBCPP_NODISCARD [[nodiscard]] #elif __has_attribute(warn_unused_result) #define _LIBCPP  __attribute__((warn_unused_result)) #else #define _LIBCPP_NODISCARD` and then  `#if (_LIBCPP_STD_VER > 17 && !_LIBCPP_DISABLE>>) || defined(_LIBCPP_FORCE_NODISCARD) #  define _LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_NODISCARD # else #  define 
[22:39:12] <thakis_> _LIBCPP_NODISCARD_AFTER_CXX17 #endif`

[lebedev.ri]

Split _LIBCPP_NODISCARD_AFTER_CXX17 defining into two defines, to avoid combinatorial explosion as per @thakis.

[thakis]

I’d expect _LIBCPP_FORCE_NODISCARD to win over _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17, but /shrug. Thanks!

[• Quuxplusone]

test/libcxx/diagnostics/force_nodiscard.fail.cpp

15 ↗

(On Diff #140679)

What is the purpose of _LIBCPP_FORCE_NODISCARD?
On one of your other nodiscard-related reviews, @EricWF suggested that we should warn all the time, even e.g. on a discarded std::move(x) in C++11, or a discarded vec.empty() in C++03. And *maybe* we could provide an opt-out mechanism, but honestly *I* don't see why anyone would want to opt out.
_LIBCPP_FORCE_NODISCARD smells like an opt-in mechanism, which I would think is the opposite of what we want.

test/libcxx/diagnostics/force_nodiscard.pass.cpp

16 ↗

(On Diff #140679)

What is the purpose of _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17? I guess I could understand a blanket opt-in "please don't warn me about discarded [[nodiscard]] results"; but that should be (and is) spelled -Wno-unused-result, and it has nothing to do with C++17.

I like how this patch defines _LIBCPP_NODISCARD in non-C++17 modes; that's going to be very useful. But I think all these opt-in mechanisms are confusing and not-helpful.

If we must have an opt-in/out mechanism (which I don't believe we do), please consider adding the following two lines to <__config> and removing the rest:

#ifdef _LIBCPP_NODISCARD
    // the user has given us their preferred spelling; use it unconditionally
#elif __has_cpp_attribute(nodiscard) && _LIBCPP_STD_VER > 17
    [... etc etc ...]

[lebedev.ri]

test/libcxx/diagnostics/force_nodiscard.fail.cpp

15 ↗

(On Diff #140679)

_LIBCPP_FORCE_NODISCARD smells like an opt-in mechanism

Correct.

And *maybe* we could provide an opt-out mechanism, but honestly *I* don't see why anyone would want to opt out.

I agree.

which I would think is the opposite of what we want.

Also correct.

test/libcxx/diagnostics/force_nodiscard.pass.cpp

16 ↗

(On Diff #140679)

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.
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.

#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 (who wants to enable nodiscard attribute for pre-C++17/whatever) out there?
I really don't see how that would be better.

[• Quuxplusone]

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.

[lebedev.ri]

In D45179#1056108, @Quuxplusone wrote:

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?

That particular disscussion was in IRC, so no, i can not link/paste.

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. :)

:)

[rjmccall]

Is Marshall arguing that the standard doesn't allow compilers to warn about failing to use these function results prior to C++17? Because I don't think that's true; warnings are thoroughly non-normative.

If libc++ wants to allow its users to opt out of these warnings just for libc++ functions, that's fine (although unusual), but it seems to me that that ought to be an explicit opt-out that's independent of language version.

[lebedev.ri]

In D45179#1056183, @rjmccall wrote:

Is Marshall arguing that the standard doesn't allow compilers to warn about failing to use these function results prior to C++17?

No. He was simply saying that people are opposed to having nodiscard attribute specified when the standard does not require that yet.
For same reason, this has to be an opt-in (_LIBCPP_FORCE_NODISCARD), it can't be just an opt-out.
But, you might as well just ask him that.

[EricWF]

I'm also in favor of making this opt-out instead of opt-in, except for a handful of functions like unique_ptr::release which may yield false positives. However, for functions where discarding the return value is always a bug, I don't see an issue in libc++ emitting a warning where it didn't previously.

[rjmccall]

Yes, if we think that the committee is likely to include questionable functions on the [[nodiscard]] list which we don't want to warn about pre-C++17, then it makes sense to have two internal macros, one for functions like std::move that should be unconditionally warned about and one for the iffy-bu-standard-directed cases.

[lebedev.ri]

Based on IRC disscussion, try to update the patch.

The main problem is that gcc does not silence the warning produced by
these attributes via (void) cast, unless it's [[nodiscard]] attribute :/
https://godbolt.org/g/m3gPKQ

So until they fix that (they will fix that right? :)),
we can only support _LIBCPP_FORCE_NODISCARD for clang compilers.

[• Quuxplusone]

In D45179#1056706, @lebedev.ri wrote:

gcc does not silence the warning produced by
these attributes via (void) cast, unless it's [[nodiscard]] attribute :/
[...] So until they fix that (they will fix that right? :)),
we can only support _LIBCPP_FORCE_NODISCARD for clang compilers.

The GCC warn_unused_result issue is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509 and also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 . I would certainly love for them to fix it, but the sentiment expressed in those bug reports is "no! never! we love being broken!". :/
I re-suggest my idea that perhaps libc++ should respect the user's choice to pass -D_LIBCPP_NODISCARD=__attribute__((warn_unused_result)) on the GCC command line. This would make your patch only one line longer than it is right now...

[lebedev.ri]

I'm waiting for @mclow.lists to have the final say re this differential.

In D45179#1057187, @Quuxplusone wrote:

In D45179#1056706, @lebedev.ri wrote:

gcc does not silence the warning produced by
these attributes via (void) cast, unless it's [[nodiscard]] attribute :/
[...] So until they fix that (they will fix that right? :)),
we can only support _LIBCPP_FORCE_NODISCARD for clang compilers.

The GCC warn_unused_result issue is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509 and also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 . I would certainly love for them to fix it, but the sentiment expressed in those bug reports is "no! never! we love being broken!". :/
I re-suggest my idea that perhaps libc++ should respect the user's choice to pass -D_LIBCPP_NODISCARD=__attribute__((warn_unused_result)) on the GCC command line. This would make your patch only one line longer than it is right now...

So roughly:

// NOTE: Do not use [[nodiscard]] in pre-C++17 mode
//       to avoid -Wc++17-extensions warning.
// And we can't use GCC's [[gnu::warn_unused_result]] and
// __attribute__((warn_unused_result)),
// because GCC does not silence them via (void) cast.
#if !defined(_LIBCPP_NODISCARD)
#  if __has_cpp_attribute(nodiscard) && _LIBCPP_STD_VER > 17
#    define _LIBCPP_NODISCARD [[nodiscard]]
#  elif __has_cpp_attribute(clang::warn_unused_result)
#    define _LIBCPP_NODISCARD [[clang::warn_unused_result]]
#  else
#    define _LIBCPP_NODISCARD
#  endif
#endif

?

[• Quuxplusone]

In D45179#1064589, @lebedev.ri wrote:

I'm waiting for @mclow.lists to have the final say re this differential.

Ack. :)

So roughly:

// NOTE: Do not use [[nodiscard]] in pre-C++17 mode
//       to avoid -Wc++17-extensions warning.
// And we can't use GCC's [[gnu::warn_unused_result]] and
// __attribute__((warn_unused_result)),
// because GCC does not silence them via (void) cast.
#if !defined(_LIBCPP_NODISCARD)
#  if __has_cpp_attribute(nodiscard) && _LIBCPP_STD_VER > 17
#    define _LIBCPP_NODISCARD [[nodiscard]]
#  elif __has_cpp_attribute(clang::warn_unused_result)
#    define _LIBCPP_NODISCARD [[clang::warn_unused_result]]
#  else
#    define _LIBCPP_NODISCARD
#  endif
#endif

?

Yes, that correctly expresses my intent. Personally I would write it like this to reduce the nesting and linecount:

// NOTE: Do not use [[nodiscard]] in pre-C++17 mode
//       to avoid -Wc++17-extensions warning.
// And we can't use GCC's [[gnu::warn_unused_result]] and
// __attribute__((warn_unused_result)),
// because GCC does not silence them via (void) cast.
#if defined(_LIBCPP_NODISCARD)
#elif __has_cpp_attribute(nodiscard) && _LIBCPP_STD_VER > 17
#  define _LIBCPP_NODISCARD [[nodiscard]]
#elif __has_cpp_attribute(clang::warn_unused_result)
#  define _LIBCPP_NODISCARD [[clang::warn_unused_result]]
#else
#  define _LIBCPP_NODISCARD
#endif

[mclow.lists]

In D45179#1056183, @rjmccall wrote:

Is Marshall arguing that the standard doesn't allow compilers to warn about failing to use these function results prior to C++17? Because I don't think that's true; warnings are thoroughly non-normative.

I have proposed to add different forms of "nodiscard" warnings to libc++ in the past, and received strong pushback from people who:

• Compile with -Werror
• Don't want to change their code.

I'm not willing to have that argument again.

[thakis]

So you're happy with this opt-in version?

[rjmccall]

In D45179#1077048, @mclow.lists wrote:

In D45179#1056183, @rjmccall wrote:

Is Marshall arguing that the standard doesn't allow compilers to warn about failing to use these function results prior to C++17? Because I don't think that's true; warnings are thoroughly non-normative.

I have proposed to add different forms of "nodiscard" warnings to libc++ in the past, and received strong pushback from people who:

• Compile with -Werror
• Don't want to change their code.

I'm not willing to have that argument again.

Alright, well, I won't push you on it.

[mclow.lists]

In D45179#1077055, @thakis wrote:

So you're happy with this opt-in version?

I'm happy with an opt-in mechanism, yes.
This one is not quite right yet.

BTW, I expect a large set of calls in the standard library to get marked as [[nodiscard]] in Rapperswil.

include/__config

1057

[[nodiscard]] is a C++17 feature. This test should be >=, not >.

1067

I wouldn't change this; just leave it as [[nodiscard]]

[lebedev.ri]

include/__config

1057

Indeed, though the left hand side of the diff already specified _LIBCPP_STD_VER > 17.
Interesting, isn't it.

1067

But that would defy the purpose?

[mclow.lists]

include/__config

1057

That was because the flag _LIBCPP_NODISCARD_AFTER_CXX17 was only active post-c++17.

Now we're talking about doing stuff in earlier standards, we should use [[nodiscard]] for C++17

1067

That's a fair point - but I don't want this to be anything but [[nodiscard]] if we're compiling for C++2a or later.

[lebedev.ri]

include/__config

1057

Oh, right, sorry!

1067

I don't want this to be anything but [[nodiscard]] if we're compiling for C++2a or later.

Aha, i see, ok.

[lebedev.ri]

Updated based on @mclow.lists review.

test/libcxx/diagnostics/force_nodiscard.pass.cpp

16 ↗

(On Diff #140679)

I tried to do that, but completely failed to come up with testing, so i'm not going to do this here, sorry.

[mclow.lists]

test/libcxx/diagnostics/force_nodiscard.fail.cpp

22 ↗

(On Diff #143872)

Shouldn't this be just _LIBCPP_NODISCARD ?

[lebedev.ri]

test/libcxx/diagnostics/force_nodiscard.fail.cpp

22 ↗

(On Diff #143872)

I don't think so?
I thought we are intentionally testing the same macro that libc++ is using internally.

[mclow.lists]

test/libcxx/diagnostics/force_nodiscard.fail.cpp

22 ↗

(On Diff #143872)

Ok, I see what you're saying.

This test is testing if a function marked _LIBCPP_NODISCARD_AFTER_CXX17 gives an error if _LIBCPP_FORCE_NODISCARD is defined.

But then you need another test, just like this, with _LIBCPP_NODISCARD int foo() { return 6; } to make sure that that gives an error as well. (and a passing test, that shows that if you don't opt-in, you get no error)

[mclow.lists]

BTW, you can gang several failing tests together, and check all the error messages - see libcxx/test/libcxx/containers/unord/unord.set/missing_hash_specialization.fail.cpp for an example.

[thakis]

What's the status here? The MS STL has this feature under _HAS_NODISCARD and it's super useful.

[lebedev.ri]

Right, sorry. Let's try to revive this.
Does this test coverage look better?
What did i miss?

[lebedev.ri]

Ping?

[EricWF]

Hijacking with permission.

[EricWF]

This updates just about everything in this patch. See the updated description for the overall design.

Specific issues addressed by this patch are:

• Rename _LIBCPP_FORCE_NODISCARD to _LIBCPP_ENABLE_NODISCARD for consistency.
• Add the _LIBCPP_NODISCARD_EXT macro to allow appling [[nodiscard]] as a pure extension.
• Document all new user facing macros and how to use them.
• Document the extension in general. (Adding a section in which libc++ should document all its extensions).
• Use [[clang::warn_unused_result]] in place of [[nodiscard]] when it's not available. All supported clang versions provide this attribute. This applies to _LIBCPP_NODISCARD_AFTER_CXX17 which may be enabled in before [[nodiscard]] is supported.
• Add more tests for all combinations.

[EricWF]

A couple other notes:

• Add an application of _LIBCPP_NODISCARD_EXT to get_temporary_buffer to demonstrate usage.

[thakis]

What's the status here? Did this land?

[EricWF]

I don't think it ever landed. I'll try to get it in this week.

[lebedev.ri]

In D45179#1225911, @EricWF wrote:

I don't think it ever landed. I'll try to get it in this week.

What does this need?
Is there some changes missing still?
Or this simply needs to be committed (and bots watched?)

[EricWF]

In D45179#1241721, @lebedev.ri wrote:

In D45179#1225911, @EricWF wrote:

I don't think it ever landed. I'll try to get it in this week.

What does this need?
Is there some changes missing still?
Or this simply needs to be committed (and bots watched?)

I think this was good to go. I was just waiting to give time for comments. Obviously I've waited long enough.

I'll work on this today.

[lebedev.ri]

In D45179#1242896, @EricWF wrote:

In D45179#1241721, @lebedev.ri wrote:

In D45179#1225911, @EricWF wrote:

I don't think it ever landed. I'll try to get it in this week.

What does this need?
Is there some changes missing still?
Or this simply needs to be committed (and bots watched?)

I think this was good to go. I was just waiting to give time for comments. Obviously I've waited long enough.

Ah, great.

I'll work on this today.

If wanted, i can commandeer this back and land.

[EricWF]

@lebedev.ri Sure, be my guest. It frees me up to do reviews. Thank you.

I just finished running the test suite, so it should be good to go.

[lebedev.ri]

In D45179#1242915, @EricWF wrote:

@lebedev.ri Sure, be my guest. It frees me up to do reviews. Thank you.

Np.

I just finished running the test suite, so it should be good to go.

Great, thank you!

Commandeering back to me, let's land this ^^

docs/UsingLibcxx.rst

**_LIBCPP_NO_VCRUNTIME**:
   prevents libc++ from depending on vcruntime headers. Consequently, it also
   prevents libc++ headers from being interoperable with vcruntime headers (from
   the aforementioned clashes), so users of this macro are promising to not
   attempt to combine libc++ headers with the problematic vcruntime headers. This
   macro also currently prevents certain `operator new`/`operator delete`
   replacement scenarios from working, e.g. replacing `operator new` and
   expecting a non-replaced `operator new[]` to call the replaced `operator new`.
 
+**_LIBCPP_ENABLE_NODISCARD**:
+  Allow the library to add ``[[nodiscard]]`` attributes to entities not specified
+  as ``[[nodiscard]]`` by the current language dialect. This includes
+  backporting applications of ``[[nodiscard]]`` from newer dialects and
+  additional extended applications at the discretion of the library. All
+  additional applications of ``[[nodiscard]]`` are disabled by default.
+  See :ref:`Extended Applications of [[nodiscard]] <nodiscard extension>` for
+  more information.
+
+**_LIBCPP_DISABLE_NODISCARD_EXT**:
+  This macro prevents the library from applying ``[[nodiscard]]`` to entities
+  purely as an extension. See :ref:`Extended Applications of [[nodiscard]] <nodiscard extension>`
+  for more information.
+
+
 C++17 Specific Configuration Macros
 -----------------------------------
 **_LIBCPP_ENABLE_CXX17_REMOVED_FEATURES**:
   This macro is used to re-enable all the features removed in C++17. The effect
   is equivalent to manually defining each macro listed below.
 
 **_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS**:
   This macro is used to re-enable the `set_unexpected`, `get_unexpected`, and
   `unexpected` functions, which were removed in C++17.
 
 **_LIBCPP_ENABLE_CXX17_REMOVED_AUTO_PTR**:
   This macro is used to re-enable `std::auto_ptr` in C++17.
+
+C++2a Specific Configuration Macros:
+------------------------------------
+**_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17**:
+  This macro can be used to disable diagnostics emitted from functions marked
+  ``[[nodiscard]]`` in dialects after C++17.  See :ref:`Extended Applications of [[nodiscard]] <nodiscard extension>`
+  for more information.
+
+
+Libc++ Extensions
+=================
+
+This section documents various extensions provided by libc++, how they're
+provided, and any information regarding how to use them.
+
+.. _nodiscard extension:
+
+Extended applications of ``[[nodiscard]]``
+------------------------------------------
+
+The ``[[nodiscard]]`` attribute is intended to help users find bugs where
+function return values are ignored when they shouldn't be. After C++17 the
+C++ standard has started to declared such library functions as ``[[nodiscard]]``.
+However, this application is limited and applies only to dialects after C++17.
+Users who want help diagnosing misuses of STL functions may desire a more
+liberal application of ``[[nodiscard]]``.
+
+For this reason libc++ provides an extension that does just that! The
+extension must be enabled by defining ``_LIBCPP_ENABLE_NODISCARD``. The extended
+applications of ``[[nodiscard]]`` takes two forms:
+
+1. Backporting ``[[nodiscard]]`` to entities declared as such by the
+   standard in newer dialects, but not in the present one.
+
+2. Extended applications of ``[[nodiscard]]``, at the libraries discretion,
+   applied to entities never declared as such by the standard.
+
+Users may also opt-out of additional applications ``[[nodiscard]]`` using
+additional macros.
+
+Applications of the first form, which backport ``[[nodiscard]]`` from a newer
+dialect may be disabled using macros specific to the dialect it was added. For
+example ``_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17``.
+
+Applications of the second form, which are pure extensions, may be disabled
+by defining ``_LIBCPP_DISABLE_NODISCARD_EXT``.
+
+
+Entities declared with ``_LIBCPP_NODISCARD_EXT``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+This section lists all extended applications of ``[[nodiscard]]`` to entities
+which no dialect declares as such (See the second form described above).
+
+* ``get_temporary_buffer``

include/__config

 #endif
 
 #if _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_CXX14_CONSTEXPR)
 #  define _LIBCPP_CONSTEXPR_AFTER_CXX17 constexpr
 #else
 #  define _LIBCPP_CONSTEXPR_AFTER_CXX17
 #endif
 
-#if __has_cpp_attribute(nodiscard) && _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17)
-#  define _LIBCPP_NODISCARD_AFTER_CXX17 [[nodiscard]]
+// The _LIBCPP_NODISCARD_ATTRIBUTE should only be used to define other
+// NODISCARD macros to the correct attribute.
+#if __has_cpp_attribute(nodiscard) || defined(_LIBCPP_COMPILER_MSVC)
+#  define _LIBCPP_NODISCARD_ATTRIBUTE [[nodiscard]]
+#elif defined(_LIBCPP_COMPILER_CLANG) && !defined(_LIBCPP_CXX03_LANG)
+#  define _LIBCPP_NODISCARD_ATTRIBUTE [[clang::warn_unused_result]]
+#else
+// We can't use GCC's [[gnu::warn_unused_result]] and
+// __attribute__((warn_unused_result)), because GCC does not silence them via
+// (void) cast.
+#  define _LIBCPP_NODISCARD_ATTRIBUTE
+#endif
+
+// _LIBCPP_NODISCARD_EXT may be used to apply [[nodiscard]] to entities not
+// specified as such as an extension.
+#if defined(_LIBCPP_ENABLE_NODISCARD) && !defined(_LIBCPP_DISABLE_NODISCARD_EXT)
+#  define _LIBCPP_NODISCARD_EXT _LIBCPP_NODISCARD_ATTRIBUTE
+#else
+#  define _LIBCPP_NODISCARD_EXT
+#endif
+
+#if !defined(_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17) && \
+    (_LIBCPP_STD_VER > 17 || defined(_LIBCPP_ENABLE_NODISCARD))
+#  define _LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_NODISCARD_ATTRIBUTE
 #else
 #  define _LIBCPP_NODISCARD_AFTER_CXX17
 #endif
 
 #if _LIBCPP_STD_VER > 14 && defined(__cpp_inline_variables) && (__cpp_inline_variables >= 201606L)
 #  define _LIBCPP_INLINE_VAR inline
 #else
 #  define _LIBCPP_INLINE_VAR

include/memory

#endif
     _LIBCPP_INLINE_VISIBILITY raw_storage_iterator  operator++(int)
         {raw_storage_iterator __t(*this); ++__x_; return __t;}
 #if _LIBCPP_STD_VER >= 14
     _LIBCPP_INLINE_VISIBILITY _OutputIterator base() const { return __x_; }
 #endif
 };
 
 template <class _Tp>
-_LIBCPP_NO_CFI
+_LIBCPP_NODISCARD_EXT _LIBCPP_NO_CFI
 pair<_Tp*, ptrdiff_t>
 get_temporary_buffer(ptrdiff_t __n) _NOEXCEPT
 {
     pair<_Tp*, ptrdiff_t> __r(0, 0);
     const ptrdiff_t __m = (~ptrdiff_t(0) ^
                            ptrdiff_t(ptrdiff_t(1) << (sizeof(ptrdiff_t) * __CHAR_BIT__ - 1)))
                            / sizeof(_Tp);
     if (__n > __m)

test/libcxx/diagnostics/enable_nodiscard.fail.cpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// Test that _LIBCPP_NODISCARD_EXT and _LIBCPP_NODISCARD_AFTER_CXX17 are defined
+// to the appropriate warning-generating attribute when _LIBCPP_ENABLE_NODISCARD
+// is explicitly provided.
+
+// UNSUPPORTED: c++98, c++03
+
+// GCC 7 is the first version to introduce [[nodiscard]]
+// UNSUPPORTED: gcc-4.9, gcc-5, gcc-6
+
+// MODULES_DEFINES: _LIBCPP_ENABLE_NODISCARD
+#define _LIBCPP_ENABLE_NODISCARD
+
+#include <__config>
+
+_LIBCPP_NODISCARD_EXT int foo() { return 42; }
+_LIBCPP_NODISCARD_AFTER_CXX17 int bar() { return 42; }
+
+int main() {
+  foo(); // expected-error-re {{ignoring return value of function declared with {{'nodiscard'|warn_unused_result}} attribute}}
+  bar(); // expected-error-re {{ignoring return value of function declared with {{'nodiscard'|warn_unused_result}} attribute}}
+  (void)foo(); // OK. void casts disable the diagnostic.
+  (void)bar();
+}

test/libcxx/diagnostics/enable_nodiscard_disable_after_cxx17.fail.cpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03
+
+// GCC 7 is the first version to introduce [[nodiscard]]
+// UNSUPPORTED: gcc-4.9, gcc-5, gcc-6
+
+// Test that _LIBCPP_DISABLE_NODISCARD_EXT only disables _LIBCPP_NODISCARD_EXT
+// and not _LIBCPP_NODISCARD_AFTER_CXX17.
+
+// MODULES_DEFINES: _LIBCPP_ENABLE_NODISCARD
+// MODULES_DEFINES: _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17
+#define _LIBCPP_ENABLE_NODISCARD
+#define _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17
+#include <__config>
+
+
+_LIBCPP_NODISCARD_EXT int foo() { return 42; }
+_LIBCPP_NODISCARD_AFTER_CXX17 int bar() { return 42; }
+
+int main() {
+  foo(); // expected-error-re {{ignoring return value of function declared with {{'nodiscard'|warn_unused_result}} attribute}}
+  bar(); // OK.
+  (void)foo(); // OK.
+}

test/libcxx/diagnostics/enable_nodiscard_disable_nodiscard_ext.fail.cpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03
+
+// GCC 7 is the first version to introduce [[nodiscard]]
+// UNSUPPORTED: gcc-4.9, gcc-5, gcc-6
+
+
+// MODULES_DEFINES: _LIBCPP_ENABLE_NODISCARD
+// MODULES_DEFINES: _LIBCPP_DISABLE_NODISCARD_EXT
+#define _LIBCPP_ENABLE_NODISCARD
+#define _LIBCPP_DISABLE_NODISCARD_EXT
+#include <__config>
+
+
+_LIBCPP_NODISCARD_EXT int foo() { return 42; }
+_LIBCPP_NODISCARD_AFTER_CXX17 int bar() { return 42; }
+
+int main() {
+  bar(); // expected-error-re {{ignoring return value of function declared with {{'nodiscard'|warn_unused_result}} attribute}}
+  foo(); // OK.
+  (void)bar(); // OK.
+}

test/libcxx/diagnostics/nodiscard.fail.cpp

-// -*- C++ -*-
-//===----------------------------------------------------------------------===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is dual licensed under the MIT and the University of Illinois Open
-// Source Licenses. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-// Test that _LIBCPP_NODISCARD_AFTER_CXX17 works
-//  #define _LIBCPP_NODISCARD_AFTER_CXX17 [[nodiscard]]
-
-// UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
-// UNSUPPORTED: clang-3.3, clang-3.4, clang-3.5, clang-3.6, clang-3.7, clang-3.8
-
-#include <__config>
-
-_LIBCPP_NODISCARD_AFTER_CXX17 int foo() { return 6; }
-
-int main ()
-{
-  foo();  // expected-error {{ignoring return value of function declared with 'nodiscard' attribute}}
-}

test/libcxx/diagnostics/nodiscard.pass.cpp

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
-// Test that _LIBCPP_NODISCARD_AFTER_CXX17 works
-//  #define _LIBCPP_NODISCARD_AFTER_CXX17 [[nodiscard]]
+// Test that _LIBCPP_NODISCARD_EXT is not defined to [[nodiscard]] unless
+// explicitly enabled by _LIBCPP_ENABLE_NODISCARD
 
-// UNSUPPORTED: c++98, c++03, c++11, c++14
-
-// MODULES_DEFINES: _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17
-#define _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17
 #include <__config>
 
-_LIBCPP_NODISCARD_AFTER_CXX17 int foo() { return 6; }
+_LIBCPP_NODISCARD_EXT int foo() { return 42; }
 
-int main ()
-{
-  foo();  // no error here!
+int main() {
+  foo(); // OK.
 }

test/libcxx/diagnostics/nodiscard_aftercxx17.fail.cpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// Test that _LIBCPP_NODISCARD_AFTER_CXX17 works
+//  #define _LIBCPP_NODISCARD_AFTER_CXX17 [[nodiscard]]
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
+
+#include <__config>
+
+_LIBCPP_NODISCARD_AFTER_CXX17 int foo() { return 6; }
+
+int main ()
+{
+  foo();  // expected-error {{ignoring return value of function declared with 'nodiscard' attribute}}
+}

test/libcxx/diagnostics/nodiscard_aftercxx17.pass.cpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// Test that _LIBCPP_NODISCARD_AFTER_CXX17 is disabled whenever
+// _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17 is defined by the user.
+
+// MODULES_DEFINES: _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17
+#define _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17
+#include <__config>
+
+_LIBCPP_NODISCARD_AFTER_CXX17 int foo() { return 6; }
+
+int main ()
+{
+  foo();  // no error here!
+}

test/libcxx/diagnostics/nodiscard_extensions.fail.cpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03
+
+// Test that entities declared [[nodiscard]] as at extension by libc++, are
+// only actually declared such when _LIBCPP_ENABLE_NODISCARD is specified.
+
+// All entities to which libc++ applies [[nodiscard]] as an extension should
+// be tested here and in nodiscard_extensions.pass.cpp. They should also
+// be listed in `UsingLibcxx.rst` in the documentation for the extension.
+
+// MODULES_DEFINES: _LIBCPP_ENABLE_NODISCARD
+#define _LIBCPP_ENABLE_NODISCARD
+
+#include <memory>
+
+#include "test_macros.h"
+
+int main() {
+  {
+    // expected-error-re@+1 {{ignoring return value of function declared with {{'nodiscard'|warn_unused_result}} attribute}}
+    std::get_temporary_buffer<int>(1);
+  }
+}

test/libcxx/diagnostics/nodiscard_extensions.pass.cpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// Test that entities declared [[nodiscard]] as at extension by libc++, are
+// only actually declared such when _LIBCPP_ENABLE_NODISCARD is specified.
+
+// All entities to which libc++ applies [[nodiscard]] as an extension should
+// be tested here and in nodiscard_extensions.fail.cpp. They should also
+// be listed in `UsingLibcxx.rst` in the documentation for the extension.
+
+#include <memory>
+
+#include "test_macros.h"
+
+int main() {
+  {
+    std::get_temporary_buffer<int>(1);
+  }
+}

utils/libcxx/test/format.py

def _evaluate_fail_test(self, test, test_cxx, parsers):
         else:
             # We still need to enable certain warnings on .fail.cpp test when
             # -verify isn't enabled. Such as -Werror=unused-result. However,
             # we don't want it enabled too liberally, which might incorrectly
             # allow unrelated failure tests to 'pass'.
             #
             # Therefore, we check if the test was expected to fail because of
             # nodiscard before enabling it
-            test_str = "ignoring return value of function declared with " \
-              + "'nodiscard' attribute"
-            if test_str in contents:
+            test_str_list = ['ignoring return value', 'nodiscard', 'NODISCARD']
+            if any(test_str in contents for test_str in test_str_list):
                 test_cxx.flags += ['-Werror=unused-result']
         cmd, out, err, rc = test_cxx.compile(source_path, out=os.devnull)
         expected_rc = 0 if use_verify else 1
         if rc == expected_rc:
             return lit.Test.PASS, ''
         else:
             report = libcxx.util.makeReport(cmd, out, err, rc)
             report_msg = ('Expected compilation to fail!' if not use_verify else
                           'Expected compilation using verify to pass!')
             return lit.Test.FAIL, report + report_msg + '\n'