Skip to content

Conversation

halbi2
Copy link
Contributor

@halbi2 halbi2 commented May 13, 2025

Fixes #130656

@hulxv seems no longer to be active, this patch version fixes the errors in the test suite from #130820.

@halbi2 halbi2 requested a review from a team as a code owner May 13, 2025 01:09
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label May 13, 2025
@llvmbot
Copy link
Member

llvmbot commented May 13, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-libcxx

Author: None (halbi2)

Changes

Fixes #130656

@hulxv seems no longer to be active, this patch version fixes the errors in the test suite from #130820.


Patch is 39.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/139651.diff

8 Files Affected:

  • (modified) libcxx/include/__expected/expected.h (+2-2)
  • (modified) libcxx/test/libcxx/utilities/expected/expected.expected/and_then.mandates.verify.cpp (+8-8)
  • (added) libcxx/test/libcxx/utilities/expected/expected.expected/nodiscard.verify.cpp (+23)
  • (modified) libcxx/test/libcxx/utilities/expected/expected.expected/or_else.mandates.verify.cpp (+8-8)
  • (modified) libcxx/test/libcxx/utilities/expected/expected.expected/transform_error.mandates.verify.cpp (+8-8)
  • (modified) libcxx/test/libcxx/utilities/expected/expected.void/and_then.mandates.verify.cpp (+8-8)
  • (modified) libcxx/test/libcxx/utilities/expected/expected.void/or_else.mandates.verify.cpp (+8-8)
  • (modified) libcxx/test/libcxx/utilities/expected/expected.void/transform_error.mandates.verify.cpp (+8-8)
diff --git a/libcxx/include/__expected/expected.h b/libcxx/include/__expected/expected.h
index 0f446b870723b..1ecf46c879bd6 100644
--- a/libcxx/include/__expected/expected.h
+++ b/libcxx/include/__expected/expected.h
@@ -448,7 +448,7 @@ class __expected_base {
 };
 
 template <class _Tp, class _Err>
-class expected : private __expected_base<_Tp, _Err> {
+class [[nodiscard]] expected : private __expected_base<_Tp, _Err> {
   static_assert(!is_reference_v<_Tp> && !is_function_v<_Tp> && !is_same_v<remove_cv_t<_Tp>, in_place_t> &&
                     !is_same_v<remove_cv_t<_Tp>, unexpect_t> && !__is_std_unexpected<remove_cv_t<_Tp>>::value &&
                     __valid_std_unexpected<_Err>::value,
@@ -1377,7 +1377,7 @@ class __expected_void_base {
 
 template <class _Tp, class _Err>
   requires is_void_v<_Tp>
-class expected<_Tp, _Err> : private __expected_void_base<_Err> {
+class [[nodiscard]] expected<_Tp, _Err> : private __expected_void_base<_Err> {
   static_assert(__valid_std_unexpected<_Err>::value,
                 "[expected.void.general] A program that instantiates expected<T, E> with a E that is not a "
                 "valid argument for unexpected<E> is ill-formed");
diff --git a/libcxx/test/libcxx/utilities/expected/expected.expected/and_then.mandates.verify.cpp b/libcxx/test/libcxx/utilities/expected/expected.expected/and_then.mandates.verify.cpp
index c46ab633295c1..7c95fc19cd693 100644
--- a/libcxx/test/libcxx/utilities/expected/expected.expected/and_then.mandates.verify.cpp
+++ b/libcxx/test/libcxx/utilities/expected/expected.expected/and_then.mandates.verify.cpp
@@ -51,7 +51,7 @@ void test() {
     // U is not a specialization of std::expected
     {
       std::expected<int, int> f1(1);
-      f1.and_then(lval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<int (&)(int &)>' requested here}}
+      (void)f1.and_then(lval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<int (&)(int &)>' requested here}}
       // expected-error-re@*:* {{static assertion failed {{.*}}The result of f(value()) must be a specialization of std::expected}}
       // expected-error-re@*:* {{{{.*}}cannot be used prior to '::' because it has no members}}
       // expected-error-re@*:* {{no matching constructor for initialization of{{.*}}}}
@@ -60,7 +60,7 @@ void test() {
     // !std::is_same_v<U:error_type, E>
     {
       std::expected<int, int> f1(1);
-      f1.and_then(lval_error_type_not_same_as_int);  // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<std::expected<int, NotSameAsInt> (&)(int &)>' requested here}}
+      (void)f1.and_then(lval_error_type_not_same_as_int);  // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<std::expected<int, NotSameAsInt> (&)(int &)>' requested here}}
       // expected-error-re@*:* {{static assertion failed {{.*}}The result of f(value()) must have the same error_type as this expected}}
     }
   }
@@ -70,7 +70,7 @@ void test() {
     // U is not a specialization of std::expected
     {
       const std::expected<int, int> f1(1);
-      f1.and_then(clval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<int (&)(const int &)>' requested here}}
+      (void)f1.and_then(clval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<int (&)(const int &)>' requested here}}
       // expected-error-re@*:* {{static assertion failed {{.*}}The result of f(value()) must be a specialization of std::expected}}
       // expected-error-re@*:* {{{{.*}}cannot be used prior to '::' because it has no members}}
       // expected-error-re@*:* {{no matching constructor for initialization of{{.*}}}}
@@ -79,7 +79,7 @@ void test() {
     // !std::is_same_v<U:error_type, E>
     {
       const std::expected<int, int> f1(1);
-      f1.and_then(clval_error_type_not_same_as_int);  // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<std::expected<int, NotSameAsInt> (&)(const int &)>' requested here}}
+      (void)f1.and_then(clval_error_type_not_same_as_int);  // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<std::expected<int, NotSameAsInt> (&)(const int &)>' requested here}}
       // expected-error-re@*:* {{static assertion failed {{.*}}The result of f(value()) must have the same error_type as this expected}}
 
     }
@@ -90,7 +90,7 @@ void test() {
     // U is not a specialization of std::expected
     {
       std::expected<int, int> f1(1);
-      std::move(f1).and_then(rval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<int (&)(int &&)>' requested here}}
+      (void)std::move(f1).and_then(rval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<int (&)(int &&)>' requested here}}
       // expected-error-re@*:* {{static assertion failed {{.*}}The result of f(std::move(value())) must be a specialization of std::expected}}
       // expected-error-re@*:* {{{{.*}}cannot be used prior to '::' because it has no members}}
       // expected-error-re@*:* {{no matching constructor for initialization of{{.*}}}}
@@ -99,7 +99,7 @@ void test() {
     // !std::is_same_v<U:error_type, E>
     {
       std::expected<int, int> f1(1);
-      std::move(f1).and_then(rval_error_type_not_same_as_int); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<std::expected<int, NotSameAsInt> (&)(int &&)>' requested here}}
+      (void)std::move(f1).and_then(rval_error_type_not_same_as_int); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<std::expected<int, NotSameAsInt> (&)(int &&)>' requested here}}
       // expected-error-re@*:* {{static assertion failed {{.*}}The result of f(std::move(value())) must have the same error_type as this expected}}
     }
   }
@@ -109,7 +109,7 @@ void test() {
     // U is not a specialization of std::expected
     {
       const std::expected<int, int> f1(1);
-      std::move(f1).and_then(crval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<int (&)(const int &&)>' requested here}}
+      (void)std::move(f1).and_then(crval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<int (&)(const int &&)>' requested here}}
       // expected-error-re@*:* {{static assertion failed {{.*}}The result of f(std::move(value())) must be a specialization of std::expected}}
       // expected-error-re@*:* {{{{.*}}cannot be used prior to '::' because it has no members}}
       // expected-error-re@*:* {{no matching constructor for initialization of{{.*}}}}
@@ -118,7 +118,7 @@ void test() {
     // !std::is_same_v<U:error_type, E>
     {
       const std::expected<int, int> f1(1);
-      std::move(f1).and_then(crval_error_type_not_same_as_int); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<std::expected<int, NotSameAsInt> (&)(const int &&)>' requested here}}
+      (void)std::move(f1).and_then(crval_error_type_not_same_as_int); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::and_then<std::expected<int, NotSameAsInt> (&)(const int &&)>' requested here}}
       // expected-error-re@*:* {{static assertion failed {{.*}}The result of f(std::move(value())) must have the same error_type as this expected}}
     }
   }
diff --git a/libcxx/test/libcxx/utilities/expected/expected.expected/nodiscard.verify.cpp b/libcxx/test/libcxx/utilities/expected/expected.expected/nodiscard.verify.cpp
new file mode 100644
index 0000000000000..ea5d51cdf051c
--- /dev/null
+++ b/libcxx/test/libcxx/utilities/expected/expected.expected/nodiscard.verify.cpp
@@ -0,0 +1,23 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: std-at-least-c++23
+
+// <expected>
+
+// Test that ignoring std::expected generates [[nodiscard]] warnings.
+
+#include <expected>
+
+std::expected<int, int> returns_expected();
+std::expected<void, int> returns_expected_void();
+
+void test() {
+  returns_expected(); // expected-warning {{ignoring return value of type 'expected<int, int>'}}
+  returns_expected_void(); // expected-warning {{ignoring return value of type 'expected<void, int>'}}
+}
diff --git a/libcxx/test/libcxx/utilities/expected/expected.expected/or_else.mandates.verify.cpp b/libcxx/test/libcxx/utilities/expected/expected.expected/or_else.mandates.verify.cpp
index af1fa53307960..8064463977e48 100644
--- a/libcxx/test/libcxx/utilities/expected/expected.expected/or_else.mandates.verify.cpp
+++ b/libcxx/test/libcxx/utilities/expected/expected.expected/or_else.mandates.verify.cpp
@@ -51,7 +51,7 @@ void test() {
     // G is not a specialization of std::expected
     {
       std::expected<int, int> f1(std::unexpected<int>(1));
-      f1.or_else(lval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::or_else<int (&)(int &)>' requested here}}
+      (void)f1.or_else(lval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::or_else<int (&)(int &)>' requested here}}
       // expected-error-re@*:* {{static assertion failed {{.*}}The result of f(error()) must be a specialization of std::expected}}
       // expected-error-re@*:* {{{{.*}}cannot be used prior to '::' because it has no members}}
       // expected-error-re@*:* {{no matching constructor for initialization of{{.*}}}}
@@ -60,7 +60,7 @@ void test() {
     // !std::is_same_v<G:value_type, T>
     {
       std::expected<int, int> f1(std::unexpected<int>(1));
-      f1.or_else(lval_error_type_not_same_as_int);  // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::or_else<std::expected<NotSameAsInt, int> (&)(int &)>' requested here}}
+      (void)f1.or_else(lval_error_type_not_same_as_int);  // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::or_else<std::expected<NotSameAsInt, int> (&)(int &)>' requested here}}
       // expected-error-re@*:* {{static assertion failed {{.*}}The result of f(error()) must have the same value_type as this expected}}
     }
   }
@@ -70,7 +70,7 @@ void test() {
     // G is not a specialization of std::expected
     {
       const std::expected<int, int> f1(std::unexpected<int>(1));
-      f1.or_else(clval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::or_else<int (&)(const int &)>' requested here}}
+      (void)f1.or_else(clval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::or_else<int (&)(const int &)>' requested here}}
       // expected-error-re@*:* {{static assertion failed {{.*}}The result of f(error()) must be a specialization of std::expected}}
       // expected-error-re@*:* {{{{.*}}cannot be used prior to '::' because it has no members}}
       // expected-error-re@*:* {{no matching constructor for initialization of{{.*}}}}
@@ -79,7 +79,7 @@ void test() {
     // !std::is_same_v<G:value_type, T>
     {
       const std::expected<int, int> f1(std::unexpected<int>(1));
-      f1.or_else(clval_error_type_not_same_as_int);  // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::or_else<std::expected<NotSameAsInt, int> (&)(const int &)>' requested here}}
+      (void)f1.or_else(clval_error_type_not_same_as_int);  // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::or_else<std::expected<NotSameAsInt, int> (&)(const int &)>' requested here}}
       // expected-error-re@*:* {{static assertion failed {{.*}}The result of f(error()) must have the same value_type as this expected}}
     }
   }
@@ -89,7 +89,7 @@ void test() {
     // G is not a specialization of std::expected
     {
       std::expected<int, int> f1(std::unexpected<int>(1));
-      std::move(f1).or_else(rval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::or_else<int (&)(int &&)>' requested here}}
+      (void)std::move(f1).or_else(rval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::or_else<int (&)(int &&)>' requested here}}
       // expected-error-re@*:* {{static assertion failed {{.*}}The result of f(std::move(error())) must be a specialization of std::expected}}
       // expected-error-re@*:* {{{{.*}}cannot be used prior to '::' because it has no members}}
       // expected-error-re@*:* {{no matching constructor for initialization of{{.*}}}}
@@ -98,7 +98,7 @@ void test() {
     // !std::is_same_v<G:value_type, T>
     {
       std::expected<int, int> f1(std::unexpected<int>(1));
-      std::move(f1).or_else(rval_error_type_not_same_as_int); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::or_else<std::expected<NotSameAsInt, int> (&)(int &&)>' requested here}}
+      (void)std::move(f1).or_else(rval_error_type_not_same_as_int); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::or_else<std::expected<NotSameAsInt, int> (&)(int &&)>' requested here}}
       // expected-error-re@*:* {{static assertion failed {{.*}}The result of f(std::move(error())) must have the same value_type as this expected}}
     }
   }
@@ -108,7 +108,7 @@ void test() {
     // G is not a specialization of std::expected
     {
       const std::expected<int, int> f1(std::unexpected<int>(1));
-      std::move(f1).or_else(crval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::or_else<int (&)(const int &&)>' requested here}}
+      (void)std::move(f1).or_else(crval_return_not_std_expected); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::or_else<int (&)(const int &&)>' requested here}}
       // expected-error-re@*:* {{static assertion failed {{.*}}The result of f(std::move(error())) must be a specialization of std::expected}}
       // expected-error-re@*:* {{{{.*}}cannot be used prior to '::' because it has no members}}
       // expected-error-re@*:* {{no matching constructor for initialization of{{.*}}}}
@@ -117,7 +117,7 @@ void test() {
     // !std::is_same_v<G:value_type, T>
     {
       const std::expected<int, int> f1(std::unexpected<int>(1));
-      std::move(f1).or_else(crval_error_type_not_same_as_int); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::or_else<std::expected<NotSameAsInt, int> (&)(const int &&)>' requested here}}
+      (void)std::move(f1).or_else(crval_error_type_not_same_as_int); // expected-note{{in instantiation of function template specialization 'std::expected<int, int>::or_else<std::expected<NotSameAsInt, int> (&)(const int &&)>' requested here}}
       // expected-error-re@*:* {{static assertion failed {{.*}}The result of f(std::move(error())) must have the same value_type as this expected}}
     }
   }
diff --git a/libcxx/test/libcxx/utilities/expected/expected.expected/transform_error.mandates.verify.cpp b/libcxx/test/libcxx/utilities/expected/expected.expected/transform_error.mandates.verify.cpp
index 61374094b7adf..4c8b4999375af 100644
--- a/libcxx/test/libcxx/utilities/expected/expected.expected/transform_error.mandates.verify.cpp
+++ b/libcxx/test/libcxx/utilities/expected/expected.expected/transform_error.mandates.verify.cpp
@@ -58,12 +58,12 @@ void test() {
   // Test & overload
   {
     std::expected<int, int> e;
-    e.transform_error(return_unexpected<int&>); // expected-error-re@*:* {{static assertion failed {{.*}}The result of {{.*}} must be a valid template argument for unexpected}}
+    (void)e.transform_error(return_unexpected<int&>); // expected-error-re@*:* {{static assertion failed {{.*}}The result of {{.*}} must be a valid template argument for unexpected}}
     // expected-error-re@*:* 0-1 {{{{(excess elements in struct initializer|no matching constructor for initialization of)}}{{.*}}}}
     // expected-error-re@*:* {{static assertion failed {{.*}}[expected.object.general] A program that instantiates the definition of template expected<T, E> for {{.*}} is ill-formed.}}
     // expected-error-re@*:* 0-1 {{union member {{.*}} has reference type {{.*}}}}
 
-    e.transform_error(return_no_object<int&>); // expected-error-re@*:* {{static assertion failed {{.*}}The result of {{.*}} must be a valid template argument for unexpected}}
+    (void)e.transform_error(return_no_object<int&>); // expected-error-re@*:* {{static assertion failed {{.*}}The result of {{.*}} must be a valid template argument for unexpected}}
     // expected-error-re@*:* 0-1 {{{{(excess elements in struct initializer|no matching constructor for initialization of)}}{{.*}}}}
     // expected-error-re@*:* {{static assertion failed {{.*}}[expected.object.general] A program that instantiates the definition of template expected<T, E> for {{.*}} is ill-formed.}}
     // expected-warning-re@*:* 0-1 {{union member {{.*}} has reference type {{.*}}, which is a Microsoft extension}}
@@ -72,27 +72,27 @@ void test() {
   // Test const& overload
   {
     const std::expected<int, int> e;
-    e.transform_error(return_unexpected<const int &>); // expected-error-re@*:* {{static assertion failed {{.*}}The result of {{.*}} must be a valid template argument for unexpected}}
+    (void)e.transform_error(return_unexpected<const int &>); // expected-error-re@*:* {{static assertion failed {{.*}}The result of {{.*}} must be a valid template argument for unexpected}}
     // expected-error-re@*:* 0-2 {{{{(excess elements in struct initializer|no matching constructor for initialization of)}}{{.*}}}}
-    e.transform_error(return_no_object<const int &>); // expected-error-re@*:* {{static assertion failed {{.*}}The result of {{.*}} must be a valid template argument for unexpected}}
+    (void)e.transform_error(return_no_object<const int &>); // expected-error-re@*:* {{static assertion failed {{.*}}The result of {{.*}} must be a valid template argument for unexpected}}
     // expected-error-re@*:* 0-2 {{{{(excess elements in struct initializer|no matching constructor for initialization of)}}{{.*}}}}
   }
 
   // Test && overload
   {
     std::expected<int, int> e;
-    std::move(e).transform_error(return_unexpected<int&&>); // expected-error-re@*:* {{static assertion failed {{.*}}The result of {{.*}} must be a valid template argument for unexpected}}
+    (void)std::move(e).transform_error(return_unexpected<int&&>); // expected-error-re@*:* {{static assertion failed {{.*}}The result of {{.*}} must be a valid template argument for unexpected}}
     // expected-error-re@*:* 0-2 {{{{(excess elements in struct initializer|no matching constructor for initialization of)}}{{.*}}}}
-    std::move(e).transform_error(return_no_object<int&&>); // expected-error-re@*:* {{static assertion failed {{.*}}The result of {{.*}} must be a valid template argument for unexpected}}
+    (void)std::move(e).transform_error(return_no_object<int&&>); // expected-error-re@*:* {{static assertion failed {{.*}}The result of {{.*}} must be a valid template argument for unexpected}}
     // expected-error-re@*:* 0-2 {{{{(excess elements in struct initializer|no matching constructor...
[truncated]

Copy link

github-actions bot commented May 13, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@philnik777 philnik777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The general consensus was that we want a way for users to allow discarding expected on a per-function basis.

@halbi2
Copy link
Contributor Author

halbi2 commented May 13, 2025

@philnik777 oh, I missed that consensus, I saw the comments from @zmodem and @hulxv about LLVM and Rust and from @strega-nil-re and @StephanTLavavej about Microsoft C++. Now there is a way to disable the warning. @huixie90 what do you think, are you willing to "give it a try?"

@philnik777
Copy link
Contributor

Now there is a way to disable the warning.

What way?

@halbi2
Copy link
Contributor Author

halbi2 commented May 26, 2025

Now there is a way to disable the warning.

What way?

The programmer can pass -D_LIBCPP_DISABLE_NODISCARD_EXPECTED=1 to disable the warning. Or do I misunderstand you, @huixie90?

@philnik777
Copy link
Contributor

Now there is a way to disable the warning.

What way?

The programmer can pass -D_LIBCPP_DISABLE_NODISCARD_EXPECTED=1 to disable the warning. Or do I misunderstand you, @huixie90?

That doesn't allow you to discard on a per-function basis.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Aug 19, 2025
halbi2 added a commit to halbi2/llvm-project that referenced this pull request Aug 22, 2025
Fulfills the requirement that @huixie90 stated in llvm#139651
that Clang should have a way to disable [[nodiscard]] on a
function by function basis.

This will allow us finally to resume fixing llvm#130656.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mark std::expected as [[nodiscard]]
3 participants