[libc++] Enable [[nodiscard]] extensions by default

philnik777 · philnik777 · commit 3c355e288188 · 2022-09-02T21:34:20.000+02:00
Adding `[[nodiscard]]` to functions is a conforming extension and done extensively in the MSVC STL. Reviewed By: ldionne, EricWF, #libc Spies: #libc_vendors, cjdb, mgrang, jloser, libcxx-commits Differential Revision: https://reviews.llvm.org/D128267
diff --git a/libcxx/docs/ReleaseNotes.rst b/libcxx/docs/ReleaseNotes.rst
@@ -75,7 +75,7 @@ API Changes
 
 - ``_LIBCPP_ENABLE_NODISCARD`` and ``_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17`` are no longer respected.
   Any standards-required ``[[nodiscard]]`` applications in C++20 are now always enabled. Any extended applications
-  can now be enabled by defining ``_LIBCPP_ENABLE_NODISCARD_EXT``.
+  are now enabled by default and can be disabled by defining ``_LIBCPP_DISABLE_NODISCARD_EXT``.
 
 ABI Affecting Changes
 ---------------------
diff --git a/libcxx/docs/UsingLibcxx.rst b/libcxx/docs/UsingLibcxx.rst
@@ -248,8 +248,8 @@ thread safety annotations.
   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_EXT**:
-  This macro allows the library to apply ``[[nodiscard]]`` to entities as an extension.
+**_LIBCPP_DISABLE_NODISCARD_EXT**:
+  This macro disables library-extensions of ``[[nodiscard]]``.
   See :ref:`Extended Applications of [[nodiscard]] <nodiscard extension>` for more information.
 
 **_LIBCPP_DISABLE_DEPRECATION_WARNINGS**:
@@ -340,8 +340,8 @@ 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_EXT``. The extended
-applications of ``[[nodiscard]]`` takes two forms:
+extension is enabled by default and can be disabled by defining ``_LIBCPP_DISABLE_NODISCARD_EXT``.
+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.
diff --git a/libcxx/include/__config b/libcxx/include/__config
@@ -858,13 +858,13 @@ _LIBCPP_BEGIN_NAMESPACE_STD _LIBCPP_END_NAMESPACE_STD
 
 // _LIBCPP_NODISCARD_EXT may be used to apply [[nodiscard]] to entities not
 // specified as such as an extension.
-#  if defined(_LIBCPP_ENABLE_NODISCARD_EXT)
+#  if !defined(_LIBCPP_DISABLE_NODISCARD_EXT)
 #    define _LIBCPP_NODISCARD_EXT _LIBCPP_NODISCARD
 #  else
 #    define _LIBCPP_NODISCARD_EXT
 #  endif
 
-#  if _LIBCPP_STD_VER > 17 || defined(_LIBCPP_ENABLE_NODISCARD_EXT)
+#  if _LIBCPP_STD_VER > 17 || !defined(_LIBCPP_DISABLE_NODISCARD_EXT)
 #    define _LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_NODISCARD
 #  else
 #    define _LIBCPP_NODISCARD_AFTER_CXX17
diff --git a/libcxx/test/libcxx/algorithms/callable.verify.cpp b/libcxx/test/libcxx/algorithms/callable.verify.cpp
@@ -24,7 +24,7 @@ int main() {
   };
 
   S a[] = {1, 2, 3, 4};
-  std::lower_bound(a, a + 4, 0, &S::compare); // expected-error@*:* {{The comparator has to be callable}}
-  std::minmax({S{1}}, &S::compare); // expected-error@*:* {{The comparator has to be callable}}
-  std::minmax_element(a, a + 4, &S::compare); // expected-error@*:* {{The comparator has to be callable}}
+  (void) std::lower_bound(a, a + 4, 0, &S::compare); // expected-error@*:* {{The comparator has to be callable}}
+  (void) std::minmax({S{1}}, &S::compare); // expected-error@*:* {{The comparator has to be callable}}
+  (void) std::minmax_element(a, a + 4, &S::compare); // expected-error@*:* {{The comparator has to be callable}}
 }
diff --git a/libcxx/test/libcxx/diagnostics/enable_nodiscard_ext.verify.cpp b/libcxx/test/libcxx/diagnostics/enable_nodiscard_ext.verify.cpp
diff --git a/libcxx/test/libcxx/diagnostics/nodiscard.pass.cpp b/libcxx/test/libcxx/diagnostics/nodiscard.pass.cpp
@@ -6,12 +6,12 @@
 //
 //===----------------------------------------------------------------------===//
 
-// Test that _LIBCPP_NODISCARD_EXT is not defined to [[nodiscard]] unless
-// explicitly enabled by _LIBCPP_ENABLE_NODISCARD
+// Test that _LIBCPP_NODISCARD_EXT is not defined to [[nodiscard]] when
+// _LIBCPP_DISABLE_NODISCARD_EXT is defined
 
-#include <__config>
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_NODISCARD_EXT
 
-#include "test_macros.h"
+#include <__config>
 
 _LIBCPP_NODISCARD_EXT int foo() { return 42; }
 
diff --git a/libcxx/test/libcxx/diagnostics/nodiscard_aftercxx17.verify.cpp b/libcxx/test/libcxx/diagnostics/nodiscard_aftercxx17.verify.cpp
@@ -9,7 +9,8 @@
 // Test that _LIBCPP_NODISCARD_AFTER_CXX17 works
 // #define _LIBCPP_NODISCARD_AFTER_CXX17 [[nodiscard]]
 
-// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: c++03
+// UNSUPPORTED: (c++11 || c++14 || c++17) && !stdlib=libc++
 
 #include <__config>
 
diff --git a/libcxx/test/libcxx/diagnostics/nodiscard_extensions.pass.cpp b/libcxx/test/libcxx/diagnostics/nodiscard_extensions.pass.cpp
@@ -7,7 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 // Test that entities declared [[nodiscard]] as at extension by libc++, are
-// only actually declared such when _LIBCPP_ENABLE_NODISCARD_EXT is specified.
+// declared as such when _LIBCPP_DISABLE_NODISCARD_EXT is specified.
 
 // This test intentionally leaks memory, so it is unsupported under ASAN.
 // UNSUPPORTED: asan
@@ -20,7 +20,7 @@
 // trigger -Wunused-value warnings.
 // ADDITIONAL_COMPILE_FLAGS: -fno-builtin
 
-// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS -D_LIBCPP_DISABLE_NODISCARD_EXT
 
 #include <algorithm>
 #include <bit> // bit_cast
diff --git a/libcxx/test/libcxx/diagnostics/nodiscard_extensions.verify.cpp b/libcxx/test/libcxx/diagnostics/nodiscard_extensions.verify.cpp
@@ -9,13 +9,12 @@
 // UNSUPPORTED: c++03
 
 // Test that entities declared [[nodiscard]] as an extension by libc++, are
-// only actually declared such when _LIBCPP_ENABLE_NODISCARD_EXT is specified.
+// actually declared as such when _LIBCPP_DISABLE_NODISCARD_EXT is not 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.
 
-// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_NODISCARD_EXT
 // ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
 
 #include <algorithm>
diff --git a/libcxx/test/libcxx/thread/thread.lock/thread.lock.guard/nodiscard.verify.cpp b/libcxx/test/libcxx/thread/thread.lock/thread.lock.guard/nodiscard.verify.cpp
@@ -21,8 +21,6 @@
 // Test that we properly apply [[nodiscard]] to lock_guard's constructors,
 // which is a libc++ extension.
 
-// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_NODISCARD_EXT
-
 #include <mutex>
 
 int main(int, char**) {
diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.merge/ranges_inplace_merge.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.merge/ranges_inplace_merge.pass.cpp
@@ -87,9 +87,9 @@ static_assert(!HasInplaceMergeIter<R<const int*>, const int*>);
 
 template <class In, template <class> class SentWrapper, std::size_t N1, std::size_t N2>
 void testInplaceMergeImpl(std::array<int, N1> input, int midIdx, std::array<int, N2> expected) {
-  std::is_sorted(input.begin(), input.begin() + midIdx);
-  std::is_sorted(input.begin() + midIdx, input.end());
-  std::is_sorted(expected.begin(), expected.end());
+  assert(std::is_sorted(input.begin(), input.begin() + midIdx));
+  assert(std::is_sorted(input.begin() + midIdx, input.end()));
+  assert(std::is_sorted(expected.begin(), expected.end()));
 
   using Sent = SentWrapper<In>;
 
diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.min.max/requires_forward_iterator.fail.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.min.max/requires_forward_iterator.fail.cpp
@@ -22,15 +22,15 @@ int main(int, char**) {
   typedef cpp17_input_iterator<const int*> Iter;
   {
     // expected-error@*:* {{std::min_element requires a ForwardIterator}}
-    std::min_element(Iter(b), Iter(e));
+    (void) std::min_element(Iter(b), Iter(e));
   }
   {
     // expected-error@*:* {{std::max_element requires a ForwardIterator}}
-    std::max_element(Iter(b), Iter(e));
+    (void) std::max_element(Iter(b), Iter(e));
   }
   {
     // expected-error@*:* {{std::minmax_element requires a ForwardIterator}}
-    std::minmax_element(Iter(b), Iter(e));
+    (void) std::minmax_element(Iter(b), Iter(e));
   }
 
 
diff --git a/libcxx/test/std/algorithms/robust_against_proxy_iterators_lifetime_bugs.pass.cpp b/libcxx/test/std/algorithms/robust_against_proxy_iterators_lifetime_bugs.pass.cpp
@@ -663,28 +663,28 @@ TEST_CONSTEXPR_CXX20 bool test() {
 
   using I = Iter;
 
-  test(simple_in, [&](I b, I e) { std::any_of(b, e, is_neg); });
-  test(simple_in, [&](I b, I e) { std::all_of(b, e, is_neg); });
-  test(simple_in, [&](I b, I e) { std::none_of(b, e, is_neg); });
-  test(simple_in, [&](I b, I e) { std::find(b, e, T{1}); });
-  test(simple_in, [&](I b, I e) { std::find_if(b, e, is_neg); });
-  test(simple_in, [&](I b, I e) { std::find_if_not(b, e, is_neg); });
+  test(simple_in, [&](I b, I e) { (void) std::any_of(b, e, is_neg); });
+  test(simple_in, [&](I b, I e) { (void) std::all_of(b, e, is_neg); });
+  test(simple_in, [&](I b, I e) { (void) std::none_of(b, e, is_neg); });
+  test(simple_in, [&](I b, I e) { (void) std::find(b, e, T{1}); });
+  test(simple_in, [&](I b, I e) { (void) std::find_if(b, e, is_neg); });
+  test(simple_in, [&](I b, I e) { (void) std::find_if_not(b, e, is_neg); });
   // TODO: find_first_of
-  test(simple_in, [&](I b, I e) { std::adjacent_find(b, e); });
+  test(simple_in, [&](I b, I e) { (void) std::adjacent_find(b, e); });
   // TODO: mismatch
   // TODO: equal
   // TODO: lexicographical_compare
   // TODO: partition_point
-  test(sorted_in, [&](I b, I e) { std::lower_bound(b, e, x); });
-  test(sorted_in, [&](I b, I e) { std::upper_bound(b, e, x); });
-  test(sorted_in, [&](I b, I e) { std::equal_range(b, e, x); });
-  test(sorted_in, [&](I b, I e) { std::binary_search(b, e, x); });
+  test(sorted_in, [&](I b, I e) { (void) std::lower_bound(b, e, x); });
+  test(sorted_in, [&](I b, I e) { (void) std::upper_bound(b, e, x); });
+  test(sorted_in, [&](I b, I e) { (void) std::equal_range(b, e, x); });
+  test(sorted_in, [&](I b, I e) { (void) std::binary_search(b, e, x); });
   // `min`, `max` and `minmax` don't use iterators.
-  test(simple_in, [&](I b, I e) { std::min_element(b, e); });
-  test(simple_in, [&](I b, I e) { std::max_element(b, e); });
-  test(simple_in, [&](I b, I e) { std::minmax_element(b, e); });
-  test(simple_in, [&](I b, I e) { std::count(b, e, x); });
-  test(simple_in, [&](I b, I e) { std::count_if(b, e, is_neg); });
+  test(simple_in, [&](I b, I e) { (void) std::min_element(b, e); });
+  test(simple_in, [&](I b, I e) { (void) std::max_element(b, e); });
+  test(simple_in, [&](I b, I e) { (void) std::minmax_element(b, e); });
+  test(simple_in, [&](I b, I e) { (void) std::count(b, e, x); });
+  test(simple_in, [&](I b, I e) { (void) std::count_if(b, e, is_neg); });
   // TODO: search
   // TODO: search_n
   // TODO: find_end
@@ -696,27 +696,27 @@ TEST_CONSTEXPR_CXX20 bool test() {
   // TODO: is_heap_until
   // `clamp` doesn't use iterators.
   // TODO: is_permutation
-  test(simple_in, [&](I b, I e) { std::for_each(b, e, is_neg); });
+  test(simple_in, [&](I b, I e) { (void) std::for_each(b, e, is_neg); });
 #if TEST_STD_VER > 14
-  test_n(simple_in, [&](I b, size_t n) { std::for_each_n(b, n, is_neg); });
+  test_n(simple_in, [&](I b, size_t n) { (void) std::for_each_n(b, n, is_neg); });
 #endif
-  test(simple_in, [&](I b, I e) { std::copy(b, e, out); });
-  test_n(simple_in, [&](I b, size_t n) { std::copy_n(b, n, out); });
-  test(simple_in, [&](I b, I e) { std::copy_backward(b, e, out + N); });
-  test(simple_in, [&](I b, I e) { std::copy_if(b, e, out, is_neg); });
-  test(simple_in, [&](I b, I e) { std::move(b, e, out); });
-  test(simple_in, [&](I b, I e) { std::move_backward(b, e, out + N); });
-  test(simple_in, [&](I b, I e) { std::transform(b, e, out, identity); });
-  test(simple_in, [&](I b, I e) { std::generate(b, e, gen); });
-  test_n(simple_in, [&](I b, size_t n) { std::generate_n(b, n, gen); });
-  test(simple_in, [&](I b, I e) { std::remove_copy(b, e, out, x); });
-  test(simple_in, [&](I b, I e) { std::remove_copy_if(b, e, out, is_neg); });
-  test(simple_in, [&](I b, I e) { std::replace(b, e, x, y); });
-  test(simple_in, [&](I b, I e) { std::replace_if(b, e, is_neg, y); });
-  test(simple_in, [&](I b, I e) { std::replace_copy(b, e, out, x, y); });
-  test(simple_in, [&](I b, I e) { std::replace_copy_if(b, e, out, is_neg, y); });
+  test(simple_in, [&](I b, I e) { (void) std::copy(b, e, out); });
+  test_n(simple_in, [&](I b, size_t n) { (void) std::copy_n(b, n, out); });
+  test(simple_in, [&](I b, I e) { (void) std::copy_backward(b, e, out + N); });
+  test(simple_in, [&](I b, I e) { (void) std::copy_if(b, e, out, is_neg); });
+  test(simple_in, [&](I b, I e) { (void) std::move(b, e, out); });
+  test(simple_in, [&](I b, I e) { (void) std::move_backward(b, e, out + N); });
+  test(simple_in, [&](I b, I e) { (void) std::transform(b, e, out, identity); });
+  test(simple_in, [&](I b, I e) { (void) std::generate(b, e, gen); });
+  test_n(simple_in, [&](I b, size_t n) { (void) std::generate_n(b, n, gen); });
+  test(simple_in, [&](I b, I e) { (void) std::remove_copy(b, e, out, x); });
+  test(simple_in, [&](I b, I e) { (void) std::remove_copy_if(b, e, out, is_neg); });
+  test(simple_in, [&](I b, I e) { (void) std::replace(b, e, x, y); });
+  test(simple_in, [&](I b, I e) { (void) std::replace_if(b, e, is_neg, y); });
+  test(simple_in, [&](I b, I e) { (void) std::replace_copy(b, e, out, x, y); });
+  test(simple_in, [&](I b, I e) { (void) std::replace_copy_if(b, e, out, is_neg, y); });
   // TODO: swap_ranges
-  test(simple_in, [&](I b, I e) { std::reverse_copy(b, e, out); });
+  test(simple_in, [&](I b, I e) { (void) std::reverse_copy(b, e, out); });
   // TODO: rotate_copy
   // TODO: sample
   // TODO: unique_copy
@@ -727,29 +727,29 @@ TEST_CONSTEXPR_CXX20 bool test() {
   // TODO: set_intersection
   // TODO: set_symmetric_difference
   // TODO: set_union
-  test(simple_in, [&](I b, I e) { std::remove(b, e, x); });
-  test(simple_in, [&](I b, I e) { std::remove_if(b, e, is_neg); });
-  test(simple_in, [&](I b, I e) { std::reverse(b, e); });
+  test(simple_in, [&](I b, I e) { (void) std::remove(b, e, x); });
+  test(simple_in, [&](I b, I e) { (void) std::remove_if(b, e, is_neg); });
+  test(simple_in, [&](I b, I e) { (void) std::reverse(b, e); });
   // TODO: rotate
   if (!TEST_IS_CONSTANT_EVALUATED)
-    test(simple_in, [&](I b, I e) { std::shuffle(b, e, rand_gen()); });
+    test(simple_in, [&](I b, I e) { (void) std::shuffle(b, e, rand_gen()); });
   // TODO: unique
-  test(simple_in, [&](I b, I e) { std::partition(b, e, is_neg); });
+  test(simple_in, [&](I b, I e) { (void) std::partition(b, e, is_neg); });
   if (!TEST_IS_CONSTANT_EVALUATED)
-    test(simple_in, [&](I b, I e) { std::stable_partition(b, e, is_neg); });
+    test(simple_in, [&](I b, I e) { (void) std::stable_partition(b, e, is_neg); });
   if (!TEST_IS_CONSTANT_EVALUATED)
-    test(sort_test_in, [&](I b, I e) { std::sort(b, e); });
+    test(sort_test_in, [&](I b, I e) { (void) std::sort(b, e); });
   if (!TEST_IS_CONSTANT_EVALUATED)
-    test(sort_test_in, [&](I b, I e) { std::stable_sort(b, e); });
+    test(sort_test_in, [&](I b, I e) { (void) std::stable_sort(b, e); });
   // TODO: partial_sort
   // TODO: nth_element
   // TODO: inplace_merge
-  test(simple_in, [&](I b, I e) { std::make_heap(b, e); });
+  test(simple_in, [&](I b, I e) { (void) std::make_heap(b, e); });
   // TODO: push_heap
   // TODO: pop_heap
   // TODO: sort_heap
-  test(simple_in, [&](I b, I e) { std::prev_permutation(b, e); });
-  test(simple_in, [&](I b, I e) { std::next_permutation(b, e); });
+  test(simple_in, [&](I b, I e) { (void) std::prev_permutation(b, e); });
+  test(simple_in, [&](I b, I e) { (void) std::next_permutation(b, e); });
 
   // TODO: algorithms in `<numeric>`
   // TODO: algorithms in `<memory>`
diff --git a/libcxx/test/std/containers/container.node/node_handle.nodiscard.verify.cpp b/libcxx/test/std/containers/container.node/node_handle.nodiscard.verify.cpp
@@ -6,7 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: c++03
+// UNSUPPORTED: (c++11 || c++14 || c++17) && !stdlib=libc++
 
 // Make sure the various node handles mark their .empty() method with
 // [[nodiscard]] starting with C++20
@@ -16,7 +17,10 @@
 #include <unordered_map>
 #include <unordered_set>
 
+#include "test_macros.h"
+
 void test() {
+#if TEST_STD_VER >= 17
     {
         std::map<int, int>::node_type node;
         node.empty(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
@@ -25,6 +29,7 @@ void test() {
         std::multimap<int, int>::node_type node;
         node.empty(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
     }
+#endif // TEST_STD_VER >= 17
     {
         std::set<int> node;
         node.empty(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
diff --git a/libcxx/test/std/language.support/support.dynamic/ptr.launder/launder.nodiscard.verify.cpp b/libcxx/test/std/language.support/support.dynamic/ptr.launder/launder.nodiscard.verify.cpp
@@ -10,7 +10,8 @@
 
 // template <class T> constexpr T* launder(T* p) noexcept;
 
-// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: c++03, c++11, c++14
+// UNSUPPORETD: c++17 && !stdlib=libc++
 
 #include <new>
 #include <cassert>
diff --git a/libcxx/test/std/utilities/memory/ptr.align/assume_aligned.nodiscard.verify.cpp b/libcxx/test/std/utilities/memory/ptr.align/assume_aligned.nodiscard.verify.cpp
@@ -1,13 +1,12 @@
 //===----------------------------------------------------------------------===//
 //
-//                     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.
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-// UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
+// UNSUPPORTED: c++03, c++11, c++14, c++17
 
 // #include <memory>
 
