From f9bddc40cdbaca19e98e2303ab2a31b1b76439df Mon Sep 17 00:00:00 2001 From: Lang Hames Date: Tue, 5 Feb 2019 23:17:11 +0000 Subject: [PATCH] [ADT] Add a fallible_iterator wrapper. A fallible iterator is one whose increment or decrement operations may fail. This would usually be supported by replacing the ++ and -- operators with methods that return error: class MyFallibleIterator { public: // ... Error inc(); Errro dec(); // ... }; The downside of this style is that it no longer conforms to the C++ iterator concept, and can not make use of standard algorithms and features such as range-based for loops. The fallible_iterator wrapper takes an iterator written in the style above and adapts it to (mostly) conform with the C++ iterator concept. It does this by providing standard ++ and -- operator implementations, returning any errors generated via a side channel (an Error reference passed into the wrapper at construction time), and immediately jumping the iterator to a known 'end' value upon error. It also marks the Error as checked any time an iterator is compared with a known end value and found to be inequal, allowing early exit from loops without redundant error checking*. Usage looks like: MyFallibleIterator I = ..., E = ...; Error Err = Error::success(); for (auto &Elem : make_fallible_range(I, E, Err)) { // Loop body is only entered when safe. // Early exits from loop body permitted without checking Err. if (SomeCondition) return; } if (Err) // Handle error. * Since failure causes a fallible iterator to jump to end, testing that a fallible iterator is not an end value implicitly verifies that the error is a success value, and so is equivalent to an error check. Reviewers: dblaikie, rupprecht Subscribers: mgorny, dexonsmith, kristina, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D57618 llvm-svn: 353237 --- docs/ProgrammersManual.rst | 80 ++++++- include/llvm/ADT/fallible_iterator.h | 243 +++++++++++++++++++++ include/llvm/Object/Archive.h | 33 ++- lib/Object/Archive.cpp | 9 +- tools/llvm-objcopy/llvm-objcopy.cpp | 3 - unittests/ADT/CMakeLists.txt | 3 + unittests/ADT/FallibleIteratorTest.cpp | 291 +++++++++++++++++++++++++ 7 files changed, 624 insertions(+), 38 deletions(-) create mode 100644 include/llvm/ADT/fallible_iterator.h create mode 100644 unittests/ADT/FallibleIteratorTest.cpp diff --git a/docs/ProgrammersManual.rst b/docs/ProgrammersManual.rst index e2cb14b4240..2f89d9baa30 100644 --- a/docs/ProgrammersManual.rst +++ b/docs/ProgrammersManual.rst @@ -935,28 +935,86 @@ Building fallible iterators and iterator ranges The archive walking examples above retrieve archive members by index, however this requires considerable boiler-plate for iteration and error checking. We can -clean this up by using ``Error`` with the "fallible iterator" pattern. The usual -C++ iterator patterns do not allow for failure on increment, but we can -incorporate support for it by having iterators hold an Error reference through -which they can report failure. In this pattern, if an increment operation fails -the failure is recorded via the Error reference and the iterator value is set to -the end of the range in order to terminate the loop. This ensures that the -dereference operation is safe anywhere that an ordinary iterator dereference -would be safe (i.e. when the iterator is not equal to end). Where this pattern -is followed (as in the ``llvm::object::Archive`` class) the result is much -cleaner iteration idiom: +clean this up by using the "fallible iterator" pattern, which supports the +following natural iteration idiom for fallible containers like Archive: .. code-block:: c++ Error Err; for (auto &Child : Ar->children(Err)) { - // Use Child - we only enter the loop when it's valid + // Use Child - only enter the loop when it's valid + + // Allow early exit from the loop body, since we know that Err is success + // when we're inside the loop. + if (BailOutOn(Child)) + return; + ... } // Check Err after the loop to ensure it didn't break due to an error. if (Err) return Err; +To enable this idiom, iterators over fallible containers are written in a +natural style, with their ``++`` and ``--`` operators replaced with fallible +``Error inc()`` and ``Error dec()`` functions. E.g.: + +.. code-block:: c++ + + class FallibleChildIterator { + public: + FallibleChildIterator(Archive &A, unsigned ChildIdx); + Archive::Child &operator*(); + friend bool operator==(const ArchiveIterator &LHS, + const ArchiveIterator &RHS); + + // operator++/operator-- replaced with fallible increment / decrement: + Error inc() { + if (!A.childValid(ChildIdx + 1)) + return make_error(...); + ++ChildIdx; + return Error::success(); + } + + Error dec() { ... } + }; + +Instances of this kind of fallible iterator interface are then wrapped with the +fallible_iterator utility which provides ``operator++`` and ``operator--``, +returning any errors via a reference passed in to the wrapper at construction +time. The fallible_iterator wrapper takes care of (a) jumping to the end of the +range on error, and (b) marking the error as checked whenever an iterator is +compared to ``end`` and found to be inequal (in particular: this marks the +error as checked throughout the body of a range-based for loop), enabling early +exit from the loop without redundant error checking. + +Instances of the fallible iterator interface (e.g. FallibleChildIterator above) +are wrapped using the ``make_fallible_itr`` and ``make_fallible_end`` +functions. E.g.: + +.. code-block:: c++ + + class Archive { + public: + using child_iterator = fallible_iterator; + + child_iterator child_begin(Error &Err) { + return make_fallible_itr(FallibleChildIterator(*this, 0), Err); + } + + child_iterator child_end() { + return make_fallible_end(FallibleChildIterator(*this, size())); + } + + iterator_range children(Error &Err) { + return make_range(child_begin(Err), child_end()); + } + }; + +Using the fallible_iterator utility allows for both natural construction of +fallible iterators (using failing ``inc`` and ``dec`` operations) and +relatively natural use of c++ iterator/loop idioms. + .. _function_apis: More information on Error and its related utilities can be found in the diff --git a/include/llvm/ADT/fallible_iterator.h b/include/llvm/ADT/fallible_iterator.h new file mode 100644 index 00000000000..6501ad2233c --- /dev/null +++ b/include/llvm/ADT/fallible_iterator.h @@ -0,0 +1,243 @@ +//===--- fallible_iterator.h - Wrapper for fallible iterators ---*- C++ -*-===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_ADT_FALLIBLE_ITERATOR_H +#define LLVM_ADT_FALLIBLE_ITERATOR_H + +#include "llvm/ADT/PointerIntPair.h" +#include "llvm/ADT/iterator_range.h" +#include "llvm/Support/Error.h" + +#include + +namespace llvm { + +/// A wrapper class for fallible iterators. +/// +/// The fallible_iterator template wraps an underlying iterator-like class +/// whose increment and decrement operations are replaced with fallible versions +/// like: +/// +/// @code{.cpp} +/// Error inc(); +/// Error dec(); +/// @endcode +/// +/// It produces an interface that is (mostly) compatible with a traditional +/// c++ iterator, including ++ and -- operators that do not fail. +/// +/// Instances of the wrapper are constructed with an instance of the +/// underlying iterator and (for non-end iterators) a reference to an Error +/// instance. If the underlying increment/decrement operations fail, the Error +/// is returned via this reference, and the resulting iterator value set to an +/// end-of-range sentinel value. This enables the following loop idiom: +/// +/// @code{.cpp} +/// class Archive { // E.g. Potentially malformed on-disk archive +/// public: +/// fallible_iterator children_begin(Error &Err); +/// fallible_iterator children_end(); +/// iterator_range> +/// children(Error &Err) { +/// return make_range(children_begin(Err), children_end()); +/// //... +/// }; +/// +/// void walk(Archive &A) { +/// Error Err = Error::success(); +/// for (auto &C : A.children(Err)) { +/// // Loop body only entered when increment succeeds. +/// } +/// if (Err) { +/// // handle error. +/// } +/// } +/// @endcode +/// +/// The wrapper marks the referenced Error as unchecked after each increment +/// and/or decrement operation, and clears the unchecked flag when a non-end +/// value is compared against end (since, by the increment invariant, not being +/// an end value proves that there was no error, and is equivalent to checking +/// that the Error is success). This allows early exits from the loop body +/// without requiring redundant error checks. +template class fallible_iterator { +private: + template + using enable_if_struct_deref_supported = std::enable_if< + !std::is_void().operator->())>::value, + decltype(std::declval().operator->())>; + +public: + /// Construct a fallible iterator that *cannot* be used as an end-of-range + /// value. + /// + /// A value created by this method can be dereferenced, incremented, + /// decremented and compared, providing the underlying type supports it. + /// + /// The error that is passed in will be initially marked as checked, so if the + /// iterator is not used at all the Error need not be checked. + static fallible_iterator itr(Underlying I, Error &Err) { + (void)!!Err; + return fallible_iterator(std::move(I), &Err); + } + + /// Construct a fallible iteratro that can be used as an end-of-range value. + /// + /// A value created by this method can be dereferenced (if the underlying + /// value points at a valid value) and compared, but not incremented or + /// decremented. + static fallible_iterator end(Underlying I) { + return fallible_iterator(std::move(I), nullptr); + } + + /// Forward dereference to the underlying iterator. + auto operator*() -> decltype(*std::declval()) { return *I; } + + /// Forward const dereference to the underlying iterator. + auto operator*() const -> decltype(*std::declval()) { + return *I; + } + + /// Forward structure dereference to the underlying iterator (if the + /// underlying iterator supports it). + template + typename enable_if_struct_deref_supported::type operator->() { + return I.operator->(); + } + + /// Forward const structure dereference to the underlying iterator (if the + /// underlying iterator supports it). + template + typename enable_if_struct_deref_supported::type operator->() const { + return I.operator->(); + } + + /// Increment the fallible iterator. + /// + /// If the underlying 'inc' operation fails, this will set the Error value + /// and update this iterator value to point to end-of-range. + /// + /// The Error value is marked as needing checking, regardless of whether the + /// 'inc' operation succeeds or fails. + fallible_iterator &operator++() { + assert(getErrPtr() && "Cannot increment end iterator"); + if (auto Err = I.inc()) + handleError(std::move(Err)); + else + resetCheckedFlag(); + return *this; + } + + /// Decrement the fallible iterator. + /// + /// If the underlying 'dec' operation fails, this will set the Error value + /// and update this iterator value to point to end-of-range. + /// + /// The Error value is marked as needing checking, regardless of whether the + /// 'dec' operation succeeds or fails. + fallible_iterator &operator--() { + assert(getErrPtr() && "Cannot decrement end iterator"); + if (auto Err = I.dec()) + handleError(std::move(Err)); + else + resetCheckedFlag(); + return *this; + } + + /// Compare fallible iterators for equality. + /// + /// Returns true if both LHS and RHS are end-of-range values, or if both are + /// non-end-of-range values whose underlying iterator values compare equal. + /// + /// If this is a comparison between an end-of-range iterator and a + /// non-end-of-range iterator, then the Error (referenced by the + /// non-end-of-range value) is marked as checked: Since all + /// increment/decrement operations result in an end-of-range value, comparing + /// false against end-of-range is equivalent to checking that the Error value + /// is success. This flag management enables early returns from loop bodies + /// without redundant Error checks. + friend bool operator==(const fallible_iterator &LHS, + const fallible_iterator &RHS) { + // If both iterators are in the end state they compare + // equal, regardless of whether either is valid. + if (LHS.isEnd() && RHS.isEnd()) + return true; + + assert(LHS.isValid() && RHS.isValid() && + "Invalid iterators can only be compared against end"); + + bool Equal = LHS.I == RHS.I; + + // If the iterators differ and this is a comparison against end then mark + // the Error as checked. + if (!Equal) { + if (LHS.isEnd()) + (void)!!*RHS.getErrPtr(); + else + (void)!!*LHS.getErrPtr(); + } + + return Equal; + } + + /// Compare fallible iterators for inequality. + /// + /// See notes for operator==. + friend bool operator!=(const fallible_iterator &LHS, + const fallible_iterator &RHS) { + return !(LHS == RHS); + } + +private: + fallible_iterator(Underlying I, Error *Err) + : I(std::move(I)), ErrState(Err, false) {} + + Error *getErrPtr() const { return ErrState.getPointer(); } + + bool isEnd() const { return getErrPtr() == nullptr; } + + bool isValid() const { return !ErrState.getInt(); } + + void handleError(Error Err) { + *getErrPtr() = std::move(Err); + ErrState.setPointer(nullptr); + ErrState.setInt(true); + } + + void resetCheckedFlag() { + *getErrPtr() = Error::success(); + } + + Underlying I; + mutable PointerIntPair ErrState; +}; + +/// Convenience wrapper to make a fallible_iterator value from an instance +/// of an underlying iterator and an Error reference. +template +fallible_iterator make_fallible_itr(Underlying I, Error &Err) { + return fallible_iterator::itr(std::move(I), Err); +} + +/// Convenience wrapper to make a fallible_iterator end value from an instance +/// of an underlying iterator. +template +fallible_iterator make_fallible_end(Underlying E) { + return fallible_iterator::end(std::move(E)); +} + +template +iterator_range> +make_fallible_range(Underlying I, Underlying E, Error &Err) { + return make_range(make_fallible_itr(std::move(I), Err), + make_fallible_end(std::move(E))); +} + +} // end namespace llvm + +#endif // LLVM_ADT_FALLIBLE_ITERATOR_H diff --git a/include/llvm/Object/Archive.h b/include/llvm/Object/Archive.h index f244ae2ce16..c40278a4f92 100644 --- a/include/llvm/Object/Archive.h +++ b/include/llvm/Object/Archive.h @@ -15,6 +15,7 @@ #include "llvm/ADT/Optional.h" #include "llvm/ADT/StringRef.h" +#include "llvm/ADT/fallible_iterator.h" #include "llvm/ADT/iterator_range.h" #include "llvm/Object/Binary.h" #include "llvm/Support/Chrono.h" @@ -142,44 +143,38 @@ public: getAsBinary(LLVMContext *Context = nullptr) const; }; - class child_iterator { + class ChildFallibleIterator { Child C; - Error *E = nullptr; public: - child_iterator() : C(Child(nullptr, nullptr, nullptr)) {} - child_iterator(const Child &C, Error *E) : C(C), E(E) {} + ChildFallibleIterator() : C(Child(nullptr, nullptr, nullptr)) {} + ChildFallibleIterator(const Child &C) : C(C) {} const Child *operator->() const { return &C; } const Child &operator*() const { return C; } - bool operator==(const child_iterator &other) const { + bool operator==(const ChildFallibleIterator &other) const { // Ignore errors here: If an error occurred during increment then getNext // will have been set to child_end(), and the following comparison should // do the right thing. return C == other.C; } - bool operator!=(const child_iterator &other) const { + bool operator!=(const ChildFallibleIterator &other) const { return !(*this == other); } - // Code in loops with child_iterators must check for errors on each loop - // iteration. And if there is an error break out of the loop. - child_iterator &operator++() { // Preincrement - assert(E && "Can't increment iterator with no Error attached"); - ErrorAsOutParameter ErrAsOutParam(E); - if (auto ChildOrErr = C.getNext()) - C = *ChildOrErr; - else { - C = C.getParent()->child_end().C; - *E = ChildOrErr.takeError(); - E = nullptr; - } - return *this; + Error inc() { + auto NextChild = C.getNext(); + if (!NextChild) + return NextChild.takeError(); + C = std::move(*NextChild); + return Error::success(); } }; + using child_iterator = fallible_iterator; + class Symbol { const Archive *Parent; uint32_t SymbolIndex; diff --git a/lib/Object/Archive.cpp b/lib/Object/Archive.cpp index 3def30949fc..1d31feb714e 100644 --- a/lib/Object/Archive.cpp +++ b/lib/Object/Archive.cpp @@ -778,19 +778,18 @@ Archive::child_iterator Archive::child_begin(Error &Err, return child_end(); if (SkipInternal) - return child_iterator(Child(this, FirstRegularData, - FirstRegularStartOfFile), - &Err); + return child_iterator::itr( + Child(this, FirstRegularData, FirstRegularStartOfFile), Err); const char *Loc = Data.getBufferStart() + strlen(Magic); Child C(this, Loc, &Err); if (Err) return child_end(); - return child_iterator(C, &Err); + return child_iterator::itr(C, Err); } Archive::child_iterator Archive::child_end() const { - return child_iterator(Child(nullptr, nullptr, nullptr), nullptr); + return child_iterator::end(Child(nullptr, nullptr, nullptr)); } StringRef Archive::Symbol::getName() const { diff --git a/tools/llvm-objcopy/llvm-objcopy.cpp b/tools/llvm-objcopy/llvm-objcopy.cpp index b46ca9b5ec9..d8dea16c062 100644 --- a/tools/llvm-objcopy/llvm-objcopy.cpp +++ b/tools/llvm-objcopy/llvm-objcopy.cpp @@ -156,9 +156,6 @@ static Error executeObjcopyOnArchive(const CopyConfig &Config, std::vector NewArchiveMembers; Error Err = Error::success(); for (const Archive::Child &Child : Ar.children(Err)) { - // FIXME: Archive::child_iterator requires that Err be checked *during* loop - // iteration, and hence does not allow early returns. - cantFail(std::move(Err)); Expected> ChildOrErr = Child.getAsBinary(); if (!ChildOrErr) return createFileError(Ar.getFileName(), ChildOrErr.takeError()); diff --git a/unittests/ADT/CMakeLists.txt b/unittests/ADT/CMakeLists.txt index 098b6b67416..d2a35273389 100644 --- a/unittests/ADT/CMakeLists.txt +++ b/unittests/ADT/CMakeLists.txt @@ -18,6 +18,7 @@ add_llvm_unittest(ADTTests DenseSetTest.cpp DepthFirstIteratorTest.cpp EquivalenceClassesTest.cpp + FallibleIteratorTest.cpp FoldingSet.cpp FunctionExtrasTest.cpp FunctionRefTest.cpp @@ -71,4 +72,6 @@ add_llvm_unittest(ADTTests VariadicFunctionTest.cpp ) +target_link_libraries(ADTTests PRIVATE LLVMTestingSupport) + add_dependencies(ADTTests intrinsics_gen) diff --git a/unittests/ADT/FallibleIteratorTest.cpp b/unittests/ADT/FallibleIteratorTest.cpp new file mode 100644 index 00000000000..d3389744ffb --- /dev/null +++ b/unittests/ADT/FallibleIteratorTest.cpp @@ -0,0 +1,291 @@ +//===- unittests/ADT/FallibleIteratorTest.cpp - fallible_iterator.h tests -===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +#include "llvm/ADT/fallible_iterator.h" +#include "llvm/Testing/Support/Error.h" + +#include "gtest/gtest-spi.h" +#include "gtest/gtest.h" + +#include +#include + +using namespace llvm; + +namespace { + +using ItemValid = enum { ValidItem, InvalidItem }; +using LinkValid = enum { ValidLink, InvalidLink }; + +class Item { +public: + Item(ItemValid V) : V(V) {} + bool isValid() const { return V == ValidItem; } + +private: + ItemValid V; +}; + +// A utility to mock "bad collections". It supports both invalid items, +// where the dereference operator may return an Error, and bad links +// where the inc/dec operations may return an Error. +// Each element of the mock collection contains a pair of a (possibly broken) +// item and link. +using FallibleCollection = std::vector>; + +class FallibleCollectionWalker { +public: + FallibleCollectionWalker(FallibleCollection &C, unsigned Idx) + : C(C), Idx(Idx) {} + + Item &operator*() { return C[Idx].first; } + + const Item &operator*() const { return C[Idx].first; } + + Error inc() { + assert(Idx != C.size() && "Walking off end of (mock) collection"); + if (C[Idx].second == ValidLink) { + ++Idx; + return Error::success(); + } + return make_error("cant get next object in (mock) collection", + inconvertibleErrorCode()); + } + + Error dec() { + assert(Idx != 0 && "Walking off start of (mock) collection"); + --Idx; + if (C[Idx].second == ValidLink) + return Error::success(); + return make_error("cant get prev object in (mock) collection", + inconvertibleErrorCode()); + } + + friend bool operator==(const FallibleCollectionWalker &LHS, + const FallibleCollectionWalker &RHS) { + assert(&LHS.C == &RHS.C && "Comparing iterators across collectionss."); + return LHS.Idx == RHS.Idx; + } + +private: + FallibleCollection &C; + unsigned Idx; +}; + +class FallibleCollectionWalkerWithStructDeref + : public FallibleCollectionWalker { +public: + using FallibleCollectionWalker::FallibleCollectionWalker; + + Item *operator->() { return &this->operator*(); } + + const Item *operator->() const { return &this->operator*(); } +}; + +class FallibleCollectionWalkerWithFallibleDeref + : public FallibleCollectionWalker { +public: + using FallibleCollectionWalker::FallibleCollectionWalker; + + Expected operator*() { + auto &I = FallibleCollectionWalker::operator*(); + if (!I.isValid()) + return make_error("bad item", inconvertibleErrorCode()); + return I; + } + + Expected operator*() const { + const auto &I = FallibleCollectionWalker::operator*(); + if (!I.isValid()) + return make_error("bad item", inconvertibleErrorCode()); + return I; + } +}; + +TEST(FallibleIteratorTest, BasicSuccess) { + + // Check that a basic use-case involing successful iteration over a + // "FallibleCollection" works. + + FallibleCollection C({{ValidItem, ValidLink}, {ValidItem, ValidLink}}); + + FallibleCollectionWalker begin(C, 0); + FallibleCollectionWalker end(C, 2); + + Error Err = Error::success(); + for (auto &Elem : + make_fallible_range(begin, end, Err)) + EXPECT_TRUE(Elem.isValid()); + cantFail(std::move(Err)); +} + +TEST(FallibleIteratorTest, BasicFailure) { + + // Check that a iteration failure (due to the InvalidLink state on element one + // of the fallible collection) breaks out of the loop and raises an Error. + + FallibleCollection C({{ValidItem, ValidLink}, {ValidItem, InvalidLink}}); + + FallibleCollectionWalker begin(C, 0); + FallibleCollectionWalker end(C, 2); + + Error Err = Error::success(); + for (auto &Elem : + make_fallible_range(begin, end, Err)) + EXPECT_TRUE(Elem.isValid()); + + EXPECT_THAT_ERROR(std::move(Err), Failed()) << "Expected failure value"; +} + +TEST(FallibleIteratorTest, NoRedundantErrorCheckOnEarlyExit) { + + // Check that an early return from the loop body does not require a redundant + // check of Err. + + FallibleCollection C({{ValidItem, ValidLink}, {ValidItem, ValidLink}}); + + FallibleCollectionWalker begin(C, 0); + FallibleCollectionWalker end(C, 2); + + Error Err = Error::success(); + for (auto &Elem : + make_fallible_range(begin, end, Err)) { + (void)Elem; + return; + } + // Err not checked, but should be ok because we exit from the loop + // body. +} + +#if LLVM_ENABLE_ABI_BREAKING_CHECKS +TEST(FallibleIteratorTest, RegularLoopExitRequiresErrorCheck) { + + // Check that Err must be checked after a normal (i.e. not early) loop exit + // by failing to check and expecting program death (due to the unchecked + // error). + + EXPECT_DEATH( + { + FallibleCollection C({{ValidItem, ValidLink}, {ValidItem, ValidLink}}); + + FallibleCollectionWalker begin(C, 0); + FallibleCollectionWalker end(C, 2); + + Error Err = Error::success(); + for (auto &Elem : + make_fallible_range(begin, end, Err)) + (void)Elem; + }, + "Program aborted due to an unhandled Error:") + << "Normal (i.e. not early) loop exit should require an error check"; +} +#endif + +TEST(FallibleIteratorTest, RawIncrementAndDecrementBehavior) { + + // Check the exact behavior of increment / decrement. + + FallibleCollection C({{ValidItem, ValidLink}, + {ValidItem, InvalidLink}, + {ValidItem, ValidLink}, + {ValidItem, InvalidLink}}); + + { + // One increment from begin succeeds. + Error Err = Error::success(); + auto I = make_fallible_itr(FallibleCollectionWalker(C, 0), Err); + ++I; + EXPECT_THAT_ERROR(std::move(Err), Succeeded()); + } + + { + // Two increments from begin fail. + Error Err = Error::success(); + auto I = make_fallible_itr(FallibleCollectionWalker(C, 0), Err); + ++I; + EXPECT_THAT_ERROR(std::move(Err), Succeeded()); + ++I; + EXPECT_THAT_ERROR(std::move(Err), Failed()) << "Expected failure value"; + } + + { + // One decement from element three succeeds. + Error Err = Error::success(); + auto I = make_fallible_itr(FallibleCollectionWalker(C, 3), Err); + --I; + EXPECT_THAT_ERROR(std::move(Err), Succeeded()); + } + + { + // One decement from element three succeeds. + Error Err = Error::success(); + auto I = make_fallible_itr(FallibleCollectionWalker(C, 3), Err); + --I; + EXPECT_THAT_ERROR(std::move(Err), Succeeded()); + --I; + EXPECT_THAT_ERROR(std::move(Err), Failed()); + } +} + +TEST(FallibleIteratorTest, CheckStructDerefOperatorSupport) { + // Check that the fallible_iterator wrapper forwards through to the + // underlying iterator's structure dereference operator if present. + + FallibleCollection C({{ValidItem, ValidLink}, + {ValidItem, ValidLink}, + {InvalidItem, InvalidLink}}); + + FallibleCollectionWalkerWithStructDeref begin(C, 0); + + { + Error Err = Error::success(); + auto I = make_fallible_itr(begin, Err); + EXPECT_TRUE(I->isValid()); + cantFail(std::move(Err)); + } + + { + Error Err = Error::success(); + const auto I = make_fallible_itr(begin, Err); + EXPECT_TRUE(I->isValid()); + cantFail(std::move(Err)); + } +} + +TEST(FallibleIteratorTest, CheckDerefToExpectedSupport) { + + // Check that the fallible_iterator wrapper forwards value types, in + // particular llvm::Expected, correctly. + + FallibleCollection C({{ValidItem, ValidLink}, + {InvalidItem, ValidLink}, + {ValidItem, ValidLink}}); + + FallibleCollectionWalkerWithFallibleDeref begin(C, 0); + FallibleCollectionWalkerWithFallibleDeref end(C, 3); + + Error Err = Error::success(); + auto I = make_fallible_itr(begin, Err); + auto E = make_fallible_end(end); + + Expected V1 = *I; + EXPECT_THAT_ERROR(V1.takeError(), Succeeded()); + ++I; + EXPECT_NE(I, E); // Implicitly check error. + Expected V2 = *I; + EXPECT_THAT_ERROR(V2.takeError(), Failed()); + ++I; + EXPECT_NE(I, E); // Implicitly check error. + Expected V3 = *I; + EXPECT_THAT_ERROR(V3.takeError(), Succeeded()); + ++I; + EXPECT_EQ(I, E); + cantFail(std::move(Err)); +} + +} // namespace