WIP: Support: Add Expected<T>::emplaceInto()

[dexonsmith]

Summary

Expected<T>::emplaceInto() takes as an out parameter any
Optional<OtherT>, with OtherT constructible from T&&. It either
calls Optional<OtherT>::emplace() with the stored value, or calls
Optional<OtherT>::reset() if none, before returning takeError().

emplaceInto() is similar to Expected<T>::moveInto() / https://reviews.llvm.org/D112278, but works with an
Optional that contains a construct-only type that T can be moved into.

Optional<ConstructOnly> P;
if (Error E = makeConstructOnly().emplaceInto(P))
  return E;

I'm not sure this is really useful enough -- construct-only types are a
real thing, but it's not often possible to move-construct them. It's
unlikely that an Expected would contain one. Perhaps this is too
specific a use case?

Also, would this be better as a free function? (It probably doesn't make
sense to add overloads to Expected<T> for arbitrary box types... but
as a free function you could easily extend without modifying Expected.)

[dexonsmith]

BTW, I marked this as "WIP" because I'm not sure this really has legs... happy to abandon if no one thinks this is useful enough... or maybe there's a way to improve it?

I have hit places where Optional::operator=() fails but Optional::emplace() succeeds. Can't remember where off the top of my head though.

[dblaikie]

Yeah, similarly mixed feelings.

Might be at some point we would put up with using a small selection of macros (google's done something similar, begrudgingly, to handle absl::Status) to make it possible to actually create a local variable with initialization from an Expected result - I think that requires statement expressions or other shenanigans to make that work, though, which is also annoying, on top of the macros.

[dexonsmith]

In D112281#3079734, @dblaikie wrote:

Yeah, similarly mixed feelings.

Might be at some point we would put up with using a small selection of macros (google's done something similar, begrudgingly, to handle absl::Status) to make it possible to actually create a local variable with initialization from an Expected result - I think that requires statement expressions or other shenanigans to make that work, though, which is also annoying, on top of the macros.

A couple of other ideas that are implementable, but I'm not sure are really better than just doing the boilerplate:

struct ChallengingType {
  ChallengingType(std::string S, bool Flag, int I, ...);
};
Expected<std::string> makeString();
Expected<std::string> makeInt();

void f1(bool Flag) {
  Optional<ChallengingType> CT;

  // Filters arguments for `Expected<T>`, joining all errors.
  // Calls `emplace()` with moved values if there are none.
  if (Error E = CT.emplaceWithExpected(makeString(), Flag, makeInt(), ...))
    return E;

  // As above, but generalized to types other than Optional.
  // Returns Expected<X> if object returns X.
  if (Error E = emplaceWithExpected(CT, makeString(), Flag, makeInt(), ...))
    return E;

  // As above, but instead of filtering for Expected, filters for a temporary
  // lambda returned by `delayExpected()`.
  if (Error E = emplaceWithDelayedExpected(
          CT,
          delayExpected([&]() {return makeString();}),
          Flag,
          delayExpected([&]() {return makeInt();}),
          ...))
    return E;

  // As above, but call any function, not just emplace.
  if (Error E = callWithDelayedExpected(
          delayExpected([&]() {return makeString();}),
          Flag,
          delayExpected([&]() {return makeInt();}),
          ...,
          [&CT](std::string S, bool Flag, int I, ...) {
            CT.emplace(std::move(S), Flag, I, ...);
          }))
    return E;

  // ... other code ...
}

Anyway, I'll abandon this.

llvm/include/llvm/Support/Error.h

#endif
   Error moveInto(
       OtherT &Value,
       std::enable_if_t<std::is_assignable<OtherT &, T &&>::value> * = nullptr) {
     if (*this)
       Value = std::move(get());
     return takeError();
   }
 
+  /// Returns \a takeError() after emplacing the held T into \p MaybeValue, or
+  /// resetting it if there is none.
+  template <class OtherT>
+  Error
+  emplaceInto(Optional<OtherT> &MaybeValue,
+              std::enable_if_t<std::is_constructible<OtherT, T &&>::value> * =
+                  nullptr) {
+    if (*this)
+      MaybeValue.emplace(std::move(get()));
+    else
+      MaybeValue.reset();
+    return takeError();
+  }
+
   /// Check that this Expected<T> is an error of type ErrT.
   template <typename ErrT> bool errorIsA() const {
     return HasError && (*getErrorStorage())->template isA<ErrT>();
   }
 
   /// Take ownership of the stored error.
   /// After calling this the Expected<T> is in an indeterminate state that can
   /// only be safely destructed. No further calls (beside the destructor) should

llvm/unittests/Support/ErrorTest.cpp

// Check that this works with optionals too.
     EXPECT_THAT_ERROR(make(7).moveInto(MaybeV), Succeeded());
     EXPECT_EQ(MoveOnlyBox(7), MaybeV);
 
     EXPECT_THAT_ERROR(makeFailure().moveInto(MaybeV), Failed());
     EXPECT_EQ(MoveOnlyBox(7), MaybeV);
   }
 }
 
+struct ConstructOnlyBox {
+  Optional<int> Box;
+
+  explicit ConstructOnlyBox(int I) : Box(I) {}
+  ConstructOnlyBox(ConstructOnlyBox &&) = default;
+
+  ConstructOnlyBox() = delete;
+  ConstructOnlyBox &operator=(ConstructOnlyBox &&) = delete;
+
+  operator MoveOnlyBox() const {
+    return Box ? MoveOnlyBox(*Box) : MoveOnlyBox();
+  }
+
+  bool operator==(const ConstructOnlyBox &RHS) const {
+    if (bool(Box) != bool(RHS.Box))
+      return false;
+    return Box ? *Box == *RHS.Box : false;
+  }
+};
+
+TEST(Error, emplaceInto) {
+  // Use ConstructOnlyBox as the T in Expected<T>. Extract it into an Optional.
+  auto make = [](int I) -> Expected<ConstructOnlyBox> {
+    return ConstructOnlyBox(I);
+  };
+  auto makeFailure = []() -> Expected<ConstructOnlyBox> {
+    return createAnyError();
+  };
+
+  {
+    Optional<ConstructOnlyBox> V;
+
+    // Failure with no prior value.
+    EXPECT_THAT_ERROR(makeFailure().emplaceInto(V), Failed());
+    EXPECT_EQ(None, V);
+
+    // Success with no prior value.
+    EXPECT_THAT_ERROR(make(5).emplaceInto(V), Succeeded());
+    EXPECT_EQ(ConstructOnlyBox(5), V);
+
+    // Success with an existing value.
+    EXPECT_THAT_ERROR(make(7).emplaceInto(V), Succeeded());
+    EXPECT_EQ(ConstructOnlyBox(7), V);
+
+    // Failure with an existing value.
+    EXPECT_THAT_ERROR(makeFailure().emplaceInto(V), Failed());
+    EXPECT_EQ(None, V);
+  }
+
+  // Check that this works with another T.
+  {
+    Optional<MoveOnlyBox> V;
+
+    // Failure with no prior value.
+    EXPECT_THAT_ERROR(makeFailure().emplaceInto(V), Failed());
+    EXPECT_EQ(None, V);
+
+    // Success with no prior value.
+    EXPECT_THAT_ERROR(make(5).emplaceInto(V), Succeeded());
+    EXPECT_EQ(MoveOnlyBox(5), V);
+
+    // Success with an existing value.
+    EXPECT_THAT_ERROR(make(7).emplaceInto(V), Succeeded());
+    EXPECT_EQ(MoveOnlyBox(7), V);
+
+    // Failure with an existing value.
+    EXPECT_THAT_ERROR(makeFailure().emplaceInto(V), Failed());
+    EXPECT_EQ(None, V);
+  }
+}
+
 } // namespace