Remove Result<Success> in favor of Result<void>
Result<void> is the correct way to do this.
Bug: 132145659
Test: libbase and init unit tests
Change-Id: I9c2e16cee6657a1895f215016df41a8b290383c7
diff --git a/base/include/android-base/result.h b/base/include/android-base/result.h
index 8f00710..897c48f 100644
--- a/base/include/android-base/result.h
+++ b/base/include/android-base/result.h
@@ -28,9 +28,8 @@
// from when the error occurred. ResultError can be used in an ostream directly to print its
// string value.
//
-// Success is a typedef that aids in creating Result<T> that do not contain a return value.
-// Result<Success> is the correct return type for a function that either returns successfully or
-// returns an error value. Returning Success() from a function that returns Result<Success> is the
+// Result<void> is the correct return type for a function that either returns successfully or
+// returns an error value. Returning {} from a function that returns Result<void> is the
// correct way to indicate that a function without a return type has completed successfully.
//
// A successful Result<T> is constructed implicitly from any type that can be implicitly converted
@@ -164,9 +163,5 @@
template <typename T>
using Result = android::base::expected<T, ResultError>;
-// Usage: `Result<Success>` as a result type that doesn't contain a value.
-// Use `return {}` or `return Success()` to return with success.
-using Success = std::monostate;
-
} // namespace base
} // namespace android
diff --git a/base/result_test.cpp b/base/result_test.cpp
index d31e775..72f97f4 100644
--- a/base/result_test.cpp
+++ b/base/result_test.cpp
@@ -49,27 +49,6 @@
EXPECT_EQ('s', Result<std::string>("success")->data()[0]);
}
-TEST(result, result_success) {
- Result<Success> result = Success();
- ASSERT_TRUE(result);
- ASSERT_TRUE(result.has_value());
-
- EXPECT_EQ(Success(), *result);
- EXPECT_EQ(Success(), result.value());
-}
-
-TEST(result, result_success_rvalue) {
- // Success() doesn't actually create a Result<Success> object, but rather an object that can be
- // implicitly constructed into a Result<Success> object.
-
- auto MakeRvalueSuccessResult = []() -> Result<Success> { return Success(); };
- ASSERT_TRUE(MakeRvalueSuccessResult());
- ASSERT_TRUE(MakeRvalueSuccessResult().has_value());
-
- EXPECT_EQ(Success(), *MakeRvalueSuccessResult());
- EXPECT_EQ(Success(), MakeRvalueSuccessResult().value());
-}
-
TEST(result, result_void) {
Result<void> ok = {};
EXPECT_TRUE(ok);
@@ -96,7 +75,7 @@
}
TEST(result, result_error) {
- Result<Success> result = Error() << "failure" << 1;
+ Result<void> result = Error() << "failure" << 1;
ASSERT_FALSE(result);
ASSERT_FALSE(result.has_value());
@@ -105,7 +84,7 @@
}
TEST(result, result_error_empty) {
- Result<Success> result = Error();
+ Result<void> result = Error();
ASSERT_FALSE(result);
ASSERT_FALSE(result.has_value());
@@ -120,7 +99,7 @@
// definition will not know what the type, T, of the underlying Result<T> object that it would
// create is.
- auto MakeRvalueErrorResult = []() -> Result<Success> { return Error() << "failure" << 1; };
+ auto MakeRvalueErrorResult = []() -> Result<void> { return Error() << "failure" << 1; };
ASSERT_FALSE(MakeRvalueErrorResult());
ASSERT_FALSE(MakeRvalueErrorResult().has_value());
@@ -131,7 +110,7 @@
TEST(result, result_errno_error) {
constexpr int test_errno = 6;
errno = test_errno;
- Result<Success> result = ErrnoError() << "failure" << 1;
+ Result<void> result = ErrnoError() << "failure" << 1;
ASSERT_FALSE(result);
ASSERT_FALSE(result.has_value());
@@ -143,7 +122,7 @@
TEST(result, result_errno_error_no_text) {
constexpr int test_errno = 6;
errno = test_errno;
- Result<Success> result = ErrnoError();
+ Result<void> result = ErrnoError();
ASSERT_FALSE(result);
ASSERT_FALSE(result.has_value());
@@ -154,7 +133,7 @@
TEST(result, result_error_from_other_result) {
auto error_text = "test error"s;
- Result<Success> result = Error() << error_text;
+ Result<void> result = Error() << error_text;
ASSERT_FALSE(result);
ASSERT_FALSE(result.has_value());
@@ -170,7 +149,7 @@
TEST(result, result_error_through_ostream) {
auto error_text = "test error"s;
- Result<Success> result = Error() << error_text;
+ Result<void> result = Error() << error_text;
ASSERT_FALSE(result);
ASSERT_FALSE(result.has_value());
@@ -188,7 +167,7 @@
auto error_text = "test error"s;
constexpr int test_errno = 6;
errno = 6;
- Result<Success> result = ErrnoError() << error_text;
+ Result<void> result = ErrnoError() << error_text;
errno = 0;
@@ -306,9 +285,7 @@
// constructor. This is done with by disabling the forwarding reference constructor if its first
// and only type is Result<T>.
TEST(result, result_result_with_success) {
- auto return_result_result_with_success = []() -> Result<Result<Success>> {
- return Result<Success>();
- };
+ auto return_result_result_with_success = []() -> Result<Result<void>> { return Result<void>(); };
auto result = return_result_result_with_success();
ASSERT_TRUE(result);
ASSERT_TRUE(*result);
@@ -318,8 +295,8 @@
}
TEST(result, result_result_with_failure) {
- auto return_result_result_with_error = []() -> Result<Result<Success>> {
- return Result<Success>(ResultError("failure string", 6));
+ auto return_result_result_with_error = []() -> Result<Result<void>> {
+ return Result<void>(ResultError("failure string", 6));
};
auto result = return_result_result_with_error();
ASSERT_TRUE(result);