Update StatusOr to support lvalue reference value types.

Some implementation notes:
 - RValue references are not supported right now. It complicates the implementation further and there doesn't seem to be a need for it. It might be done in the future.
 - Any kind of reference-to-reference conversion only allows those that do not require temporaries to be materialized. Eg `StatusOr<int&>` can convert to `StatusOr<const int&>`, but it can't convert to `StatusOr<const double&>`.
 - `operator*`/`value()`/`value_or()` always return the reference type, regardless of qualifications of the StatusOr.

PiperOrigin-RevId: 776150069
Change-Id: I8446e7f76f6227f24e4de4b9490d20a8156ee8ab

Author: sbenzaquen

absl/status/internal/statusor_internal.h

@@ -90,17 +90,34 @@ template <typename T, typename V>
 struct IsDirectInitializationAmbiguous<T, absl::StatusOr<V>>
     : public IsConstructibleOrConvertibleFromStatusOr<T, V> {};
 
+// Checks whether the conversion from U to T can be done without dangling
+// temporaries.
+// REQUIRES: T and U are references.
+template <typename T, typename U>
+using IsReferenceConversionValid = absl::conjunction<  //
+    std::is_reference<T>, std::is_reference<U>,
+    // The references are convertible. This checks for
+    // lvalue/rvalue compatibility.
+    std::is_convertible<U, T>,
+    // The pointers are convertible. This checks we don't have
+    // a temporary.
+    std::is_convertible<std::remove_reference_t<U>*,
+                        std::remove_reference_t<T>*>>;
+
 // Checks against the constraints of the direction initialization, i.e. when
 // `StatusOr<T>::StatusOr(U&&)` should participate in overload resolution.
 template <typename T, typename U>
 using IsDirectInitializationValid = absl::disjunction<
     // Short circuits if T is basically U.
-    std::is_same<T, absl::remove_cvref_t<U>>,
-    absl::negation<absl::disjunction<
-        std::is_same<absl::StatusOr<T>, absl::remove_cvref_t<U>>,
-        std::is_same<absl::Status, absl::remove_cvref_t<U>>,
-        std::is_same<absl::in_place_t, absl::remove_cvref_t<U>>,
-        IsDirectInitializationAmbiguous<T, U>>>>;
+    std::is_same<T, absl::remove_cvref_t<U>>,  //
+    std::conditional_t<
+        std::is_reference_v<T>,  //
+        IsReferenceConversionValid<T, U>,
+        absl::negation<absl::disjunction<
+            std::is_same<absl::StatusOr<T>, absl::remove_cvref_t<U>>,
+            std::is_same<absl::Status, absl::remove_cvref_t<U>>,
+            std::is_same<absl::in_place_t, absl::remove_cvref_t<U>>,
+            IsDirectInitializationAmbiguous<T, U>>>>>;
 
 // This trait detects whether `StatusOr<T>::operator=(U&&)` is ambiguous, which
 // is equivalent to whether all the following conditions are met:
@@ -140,7 +157,9 @@ using Equality = std::conditional_t<Value, T, absl::negation<T>>;
 template <bool Explicit, typename T, typename U, bool Lifetimebound>
 using IsConstructionValid = absl::conjunction<
     Equality<Lifetimebound,
-             type_traits_internal::IsLifetimeBoundAssignment<T, U>>,
+             absl::disjunction<
+                 std::is_reference<T>,
+                 type_traits_internal::IsLifetimeBoundAssignment<T, U>>>,
     IsDirectInitializationValid<T, U&&>, std::is_constructible<T, U&&>,
     Equality<!Explicit, std::is_convertible<U&&, T>>,
     absl::disjunction<
@@ -156,8 +175,13 @@ using IsConstructionValid = absl::conjunction<
 template <typename T, typename U, bool Lifetimebound>
 using IsAssignmentValid = absl::conjunction<
     Equality<Lifetimebound,
-             type_traits_internal::IsLifetimeBoundAssignment<T, U>>,
-    std::is_constructible<T, U&&>, std::is_assignable<T&, U&&>,
+             absl::disjunction<
+                 std::is_reference<T>,
+                 type_traits_internal::IsLifetimeBoundAssignment<T, U>>>,
+    std::conditional_t<std::is_reference_v<T>,
+                       IsReferenceConversionValid<T, U&&>,
+                       absl::conjunction<std::is_constructible<T, U&&>,
+                                         std::is_assignable<T&, U&&>>>,
     absl::disjunction<
         std::is_same<T, absl::remove_cvref_t<U>>,
         absl::conjunction<
@@ -178,6 +202,9 @@ template <bool Explicit, typename T, typename U, bool Lifetimebound,
           typename UQ>
 using IsConstructionFromStatusOrValid = absl::conjunction<
     absl::negation<std::is_same<T, U>>,
+    // If `T` is a reference, then U must be a compatible one.
+    absl::disjunction<absl::negation<std::is_reference<T>>,
+                      IsReferenceConversionValid<T, U>>,
     Equality<Lifetimebound,
              type_traits_internal::IsLifetimeBoundAssignment<T, U>>,
     std::is_constructible<T, UQ>,
@@ -193,6 +220,16 @@ using IsStatusOrAssignmentValid = absl::conjunction<
     absl::negation<IsConstructibleOrConvertibleOrAssignableFromStatusOr<
         T, absl::remove_cvref_t<U>>>>;
 
+template <typename T, typename U, bool Lifetimebound>
+using IsValueOrValid = absl::conjunction<
+    // If `T` is a reference, then U must be a compatible one.
+    absl::disjunction<absl::negation<std::is_reference<T>>,
+                      IsReferenceConversionValid<T, U>>,
+    Equality<Lifetimebound,
+             absl::disjunction<
+                 std::is_reference<T>,
+                 type_traits_internal::IsLifetimeBoundAssignment<T, U>>>>;
+
 class Helper {
  public:
   // Move type-agnostic error handling to the .cc.
@@ -209,6 +246,26 @@ void PlacementNew(void* absl_nonnull p, Args&&... args) {
   new (p) T(std::forward<Args>(args)...);
 }
 
+template <typename T>
+class Reference {
+ public:
+  constexpr explicit Reference(T ref ABSL_ATTRIBUTE_LIFETIME_BOUND)
+      : payload_(std::addressof(ref)) {}
+
+  Reference(const Reference&) = default;
+  Reference& operator=(const Reference&) = default;
+  Reference& operator=(T value) {
+    payload_ = std::addressof(value);
+    return *this;
+  }
+
+  operator T() const { return static_cast<T>(*payload_); }  // NOLINT
+  T get() const { return *this; }
+
+ private:
+  std::remove_reference_t<T>* absl_nonnull payload_;
+};
+
 // Helper base class to hold the data and all operations.
 // We move all this to a base class to allow mixing with the appropriate
 // TraitsBase specialization.
@@ -217,6 +274,14 @@ class StatusOrData {
   template <typename U>
   friend class StatusOrData;
 
+  decltype(auto) MaybeMoveData() {
+    if constexpr (std::is_reference_v<T>) {
+      return data_.get();
+    } else {
+      return std::move(data_);
+    }
+  }
+
  public:
   StatusOrData() = delete;
 
@@ -231,7 +296,7 @@ class StatusOrData {
 
   StatusOrData(StatusOrData&& other) noexcept {
     if (other.ok()) {
-      MakeValue(std::move(other.data_));
+      MakeValue(other.MaybeMoveData());
       MakeStatus();
     } else {
       MakeStatus(std::move(other.status_));
@@ -251,7 +316,7 @@ class StatusOrData {
   template <typename U>
   explicit StatusOrData(StatusOrData<U>&& other) {
     if (other.ok()) {
-      MakeValue(std::move(other.data_));
+      MakeValue(other.MaybeMoveData());
       MakeStatus();
     } else {
       MakeStatus(std::move(other.status_));
@@ -264,13 +329,6 @@ class StatusOrData {
     MakeStatus();
   }
 
-  explicit StatusOrData(const T& value) : data_(value) {
-    MakeStatus();
-  }
-  explicit StatusOrData(T&& value) : data_(std::move(value)) {
-    MakeStatus();
-  }
-
   template <typename U,
             absl::enable_if_t<std::is_constructible<absl::Status, U&&>::value,
                               int> = 0>
@@ -290,7 +348,7 @@ class StatusOrData {
   StatusOrData& operator=(StatusOrData&& other) {
     if (this == &other) return *this;
     if (other.ok())
-      Assign(std::move(other.data_));
+      Assign(other.MaybeMoveData());
     else
       AssignStatus(std::move(other.status_));
     return *this;
@@ -299,7 +357,9 @@ class StatusOrData {
   ~StatusOrData() {
     if (ok()) {
       status_.~Status();
-      data_.~T();
+      if constexpr (!std::is_trivially_destructible_v<T>) {
+        data_.~T();
+      }
     } else {
       status_.~Status();
     }
@@ -340,11 +400,13 @@ class StatusOrData {
     // When T is const, we need some non-const object we can cast to void* for
     // the placement new. dummy_ is that object.
     Dummy dummy_;
-    T data_;
+    std::conditional_t<std::is_reference_v<T>, Reference<T>, T> data_;
   };
 
   void Clear() {
-    if (ok()) data_.~T();
+    if constexpr (!std::is_trivially_destructible_v<T>) {
+      if (ok()) data_.~T();
+    }
   }
 
   void EnsureOk() const {
@@ -359,7 +421,8 @@ class StatusOrData {
   // argument.
   template <typename... Arg>
   void MakeValue(Arg&&... arg) {
-    internal_statusor::PlacementNew<T>(&dummy_, std::forward<Arg>(arg)...);
+    internal_statusor::PlacementNew<decltype(data_)>(&dummy_,
+                                                     std::forward<Arg>(arg)...);
   }
 
   // Construct the status (ie. status_) through placement new with the passed
@@ -369,6 +432,22 @@ class StatusOrData {
     internal_statusor::PlacementNew<Status>(&status_,
                                             std::forward<Args>(args)...);
   }
+
+  template <typename U>
+  T ValueOrImpl(U&& default_value) const& {
+    if (ok()) {
+      return data_;
+    }
+    return std::forward<U>(default_value);
+  }
+
+  template <typename U>
+  T ValueOrImpl(U&& default_value) && {
+    if (ok()) {
+      return std::move(data_);
+    }
+    return std::forward<U>(default_value);
+  }
 };
 
 // Helper base classes to allow implicitly deleted constructors and assignment
@@ -411,8 +490,9 @@ struct MoveCtorBase<T, false> {
   MoveCtorBase& operator=(MoveCtorBase&&) = default;
 };
 
-template <typename T, bool = std::is_copy_constructible<T>::value&&
-                          std::is_copy_assignable<T>::value>
+template <typename T, bool = (std::is_copy_constructible<T>::value &&
+                              std::is_copy_assignable<T>::value) ||
+                             std::is_reference_v<T>>
 struct CopyAssignBase {
   CopyAssignBase() = default;
   CopyAssignBase(const CopyAssignBase&) = default;
@@ -430,8 +510,9 @@ struct CopyAssignBase<T, false> {
   CopyAssignBase& operator=(CopyAssignBase&&) = default;
 };
 
-template <typename T, bool = std::is_move_constructible<T>::value&&
-                          std::is_move_assignable<T>::value>
+template <typename T, bool = (std::is_move_constructible<T>::value &&
+                              std::is_move_assignable<T>::value) ||
+                             std::is_reference_v<T>>
 struct MoveAssignBase {
   MoveAssignBase() = default;
   MoveAssignBase(const MoveAssignBase&) = default;

absl/status/status_matchers_test.cc

@@ -18,6 +18,7 @@
 #include "absl/status/status_matchers.h"
 
 #include <string>
+#include <vector>
 
 #include "gmock/gmock.h"
 #include "gtest/gtest-spi.h"
@@ -31,9 +32,12 @@ namespace {
 using ::absl_testing::IsOk;
 using ::absl_testing::IsOkAndHolds;
 using ::absl_testing::StatusIs;
+using ::testing::ElementsAre;
 using ::testing::Eq;
 using ::testing::Gt;
 using ::testing::MatchesRegex;
+using ::testing::Not;
+using ::testing::Ref;
 
 TEST(StatusMatcherTest, StatusIsOk) { EXPECT_THAT(absl::OkStatus(), IsOk()); }
 
@@ -158,4 +162,23 @@ TEST(StatusMatcherTest, StatusIsFailure) {
       "ungueltig");
 }
 
+TEST(StatusMatcherTest, ReferencesWork) {
+  int i = 17;
+  int j = 19;
+  EXPECT_THAT(absl::StatusOr<int&>(i), IsOkAndHolds(17));
+  EXPECT_THAT(absl::StatusOr<int&>(i), Not(IsOkAndHolds(19)));
+  EXPECT_THAT(absl::StatusOr<const int&>(i), IsOkAndHolds(17));
+
+  // Reference testing works as expected.
+  EXPECT_THAT(absl::StatusOr<int&>(i), IsOkAndHolds(Ref(i)));
+  EXPECT_THAT(absl::StatusOr<int&>(i), Not(IsOkAndHolds(Ref(j))));
+
+  // Try a more complex one.
+  std::vector<std::string> vec = {"A", "B", "C"};
+  EXPECT_THAT(absl::StatusOr<std::vector<std::string>&>(vec),
+              IsOkAndHolds(ElementsAre("A", "B", "C")));
+  EXPECT_THAT(absl::StatusOr<std::vector<std::string>&>(vec),
+              Not(IsOkAndHolds(ElementsAre("A", "X", "C"))));
+}
+
 }  // namespace