Support: Add Expected<T>::moveInto() to avoid extra names

Expected<T>::moveInto() takes as an out parameter any `OtherT&` that's
assignable from `T&&`. It moves any stored value before returning
takeError().

Since moveInto() consumes both the Error and the value, it's only
anticipated that we'd use call it on temporaries/rvalues, with naming
the Expected first likely to be an anti-pattern of sorts (either you
want to deal with both at the same time, or you don't). As such,
starting it out as `&&`-qualified... but it'd probably be fine to drop
that if there's a good use case for lvalues that appears.

There are two common patterns that moveInto() cleans up:
```
  // If the variable is new:
  Expected<std::unique_ptr<int>> ExpectedP = makePointer();
  if (!ExpectedP)
    return ExpectedP.takeError();
  std::unique_ptr<int> P = std::move(*ExpectedP);

  // If the target variable already exists:
  if (Expected<T> ExpectedP = makePointer())
    P = std::move(*ExpectedP);
  else
    return ExpectedP.takeError();
```
moveInto() takes less typing and avoids needing to name (or leak into
the scope) an extra variable.
```
  // If the variable is new:
  std::unique_ptr<int> P;
  if (Error E = makePointer().moveInto(P))
    return E;

  // If the target variable already exists:
  if (Error E = makePointer().moveInto(P))
    return E;
```

It also seems useful for unit tests, to log errors (but continue) when
there's an unexpected failure. E.g.:
```
  // Crash on error, or undefined in non-asserts builds.
  std::unique_ptr<MemoryBuffer> MB = cantFail(makeMemoryBuffer());

  // Avoid crashing on error without moveInto() :(.
  Expected<std::unique_ptr<MemoryBuffer>>
      ExpectedMB = makeMemoryBuffer();
  ASSERT_THAT_ERROR(ExpectedMB.takeError(), Succeeded());
  std::unique_ptr<MemoryBuffer> MB = std::move(ExpectedMB);

  // Avoid crashing on error with moveInto() :).
  std::unique_ptr<MemoryBuffer> MB;
  ASSERT_THAT_ERROR(makeMemoryBuffer().moveInto(MB), Succeeded());
```

Differential Revision: https://reviews.llvm.org/D112278
This commit is contained in:
Duncan P. N. Exon Smith 2021-10-20 12:03:31 -07:00
parent 4d692daa3a
commit 27181cad0d
3 changed files with 117 additions and 0 deletions

View File

@ -568,6 +568,46 @@ rewritten as:
This second form is often more readable for functions that involve multiple
``Expected<T>`` values as it limits the indentation required.
If an ``Expected<T>`` value will be moved into an existing variable then the
``moveInto()`` method avoids the need to name an extra variable. This is
useful to enable ``operator->()`` the ``Expected<T>`` value has pointer-like
semantics. For example:
.. code-block:: c++
Expected<std::unique_ptr<MemoryBuffer>> openBuffer(StringRef Path);
Error processBuffer(StringRef Buffer);
Error processBufferAtPath(StringRef Path) {
// Try to open a buffer.
std::unique_ptr<MemoryBuffer> MB;
if (auto Err = openBuffer(Path).moveInto(MB))
// On error, return the Error value.
return Err;
// On success, use MB.
return processContent(MB->getBuffer());
}
This third form works with any type that can be assigned to from ``T&&``. This
can be useful if the ``Expected<T>`` value needs to be stored an already-declared
``Optional<T>``. For example:
.. code-block:: c++
Expected<StringRef> extractClassName(StringRef Definition);
struct ClassData {
StringRef Definition;
Optional<StringRef> LazyName;
...
Error initialize() {
if (auto Err = extractClassName(Path).moveInto(LazyName))
// On error, return the Error value.
return Err;
// On success, LazyName has been initialized.
...
}
};
All ``Error`` instances, whether success or failure, must be either checked or
moved from (via ``std::move`` or a return) before they are destructed.
Accidentally discarding an unchecked error will cause a program abort at the

View File

@ -577,6 +577,16 @@ public:
return const_cast<Expected<T> *>(this)->get();
}
/// Returns \a takeError() after moving the held T (if any) into \p V.
template <class OtherT>
Error moveInto(OtherT &Value,
std::enable_if_t<std::is_assignable<OtherT &, T &&>::value> * =
nullptr) && {
if (*this)
Value = std::move(get());
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>();

View File

@ -1032,4 +1032,71 @@ TEST(Error, SubtypeStringErrorTest) {
"Error 2.");
}
static Error createAnyError() {
return errorCodeToError(test_error_code::unspecified);
}
struct MoveOnlyBox {
Optional<int> Box;
explicit MoveOnlyBox(int I) : Box(I) {}
MoveOnlyBox() = default;
MoveOnlyBox(MoveOnlyBox &&) = default;
MoveOnlyBox &operator=(MoveOnlyBox &&) = default;
MoveOnlyBox(const MoveOnlyBox &) = delete;
MoveOnlyBox &operator=(const MoveOnlyBox &) = delete;
bool operator==(const MoveOnlyBox &RHS) const {
if (bool(Box) != bool(RHS.Box))
return false;
return Box ? *Box == *RHS.Box : false;
}
};
TEST(Error, moveInto) {
// Use MoveOnlyBox as the T in Expected<T>.
auto make = [](int I) -> Expected<MoveOnlyBox> { return MoveOnlyBox(I); };
auto makeFailure = []() -> Expected<MoveOnlyBox> { return createAnyError(); };
{
MoveOnlyBox V;
// Failure with no prior value.
EXPECT_THAT_ERROR(makeFailure().moveInto(V), Failed());
EXPECT_EQ(None, V.Box);
// Success with no prior value.
EXPECT_THAT_ERROR(make(5).moveInto(V), Succeeded());
EXPECT_EQ(5, V.Box);
// Success with an existing value.
EXPECT_THAT_ERROR(make(7).moveInto(V), Succeeded());
EXPECT_EQ(7, V.Box);
// Failure with an existing value. Might be nice to assign a
// default-constructed value in this case, but for now it's being left
// alone.
EXPECT_THAT_ERROR(makeFailure().moveInto(V), Failed());
EXPECT_EQ(7, V.Box);
}
// Check that this works with optionals too.
{
// Same cases as above.
Optional<MoveOnlyBox> MaybeV;
EXPECT_THAT_ERROR(makeFailure().moveInto(MaybeV), Failed());
EXPECT_EQ(None, MaybeV);
EXPECT_THAT_ERROR(make(5).moveInto(MaybeV), Succeeded());
EXPECT_EQ(MoveOnlyBox(5), MaybeV);
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);
}
}
} // namespace