-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[libc++] Mark std::expected as nodiscard #139651
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-clang @llvm/pr-subscribers-libcxx Author: None (halbi2) ChangesFixes #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:
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]
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this 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.
@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?" |
What way? |
The programmer can pass |
That doesn't allow you to discard on a per-function basis. |
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.
Fixes #130656
@hulxv seems no longer to be active, this patch version fixes the errors in the test suite from #130820.