[llvm] r294349 - ADT: Add explicit conversions for reverse ilist iterators
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 7 14:48:58 PST 2017
Hans, can we take this commit (r294349) and David's fixup in r294357 into the LLVM 4.0 branch?
I've also attached a patch for the release notes that attempt to summarize the ilist changes.
Thanks!
Duncan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Release-notes-for-ilist-changes.patch
Type: application/octet-stream
Size: 4734 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170207/6d7adaca/attachment.obj>
-------------- next part --------------
> On 2017-Feb-07, at 13:03, Duncan P. N. Exon Smith via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> Author: dexonsmith
> Date: Tue Feb 7 15:03:50 2017
> New Revision: 294349
>
> URL: http://llvm.org/viewvc/llvm-project?rev=294349&view=rev
> Log:
> ADT: Add explicit conversions for reverse ilist iterators
>
> Add explicit conversions between forward and reverse ilist iterators.
> These follow the conversion conventions of std::reverse_iterator, which
> are off-by-one: the newly-constructed "reverse" iterator dereferences to
> the previous node of the one sent in. This has the benefit of
> converting reverse ranges in place:
> - If [I, E) is a valid range,
> - then [reverse(E), reverse(I)) gives the same range in reverse order.
>
> ilist_iterator::getReverse() is unchanged: it returns a reverse iterator
> to the *same* node.
>
> Modified:
> llvm/trunk/include/llvm/ADT/ilist_iterator.h
> llvm/trunk/include/llvm/CodeGen/MachineInstrBundleIterator.h
> llvm/trunk/unittests/ADT/IListIteratorTest.cpp
> llvm/trunk/unittests/CodeGen/MachineInstrBundleIteratorTest.cpp
>
> Modified: llvm/trunk/include/llvm/ADT/ilist_iterator.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ilist_iterator.h?rev=294349&r1=294348&r2=294349&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/ilist_iterator.h (original)
> +++ llvm/trunk/include/llvm/ADT/ilist_iterator.h Tue Feb 7 15:03:50 2017
> @@ -102,10 +102,23 @@ public:
> return *this;
> }
>
> - /// Convert from an iterator to its reverse.
> + /// Explicit conversion between forward/reverse iterators.
> ///
> - /// TODO: Roll this into the implicit constructor once we're sure that no one
> - /// is relying on the std::reverse_iterator off-by-one semantics.
> + /// Translate between forward and reverse iterators without changing range
> + /// boundaries. The resulting iterator will dereference (and have a handle)
> + /// to the previous node, which is somewhat unexpected; but converting the
> + /// two endpoints in a range will give the same range in reverse.
> + ///
> + /// This matches std::reverse_iterator conversions.
> + explicit ilist_iterator(
> + const ilist_iterator<OptionsT, !IsReverse, IsConst> &RHS)
> + : ilist_iterator(++RHS.getReverse()) {}
> +
> + /// Get a reverse iterator to the same node.
> + ///
> + /// Gives a reverse iterator that will dereference (and have a handle) to the
> + /// same node. Converting the endpoint iterators in a range will give a
> + /// different range; for range operations, use the explicit conversions.
> ilist_iterator<OptionsT, !IsReverse, IsConst> getReverse() const {
> if (NodePtr)
> return ilist_iterator<OptionsT, !IsReverse, IsConst>(*NodePtr);
>
> Modified: llvm/trunk/include/llvm/CodeGen/MachineInstrBundleIterator.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineInstrBundleIterator.h?rev=294349&r1=294348&r2=294349&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/MachineInstrBundleIterator.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/MachineInstrBundleIterator.h Tue Feb 7 15:03:50 2017
> @@ -153,6 +153,18 @@ public:
> : MII(I.getInstrIterator()) {}
> MachineInstrBundleIterator() : MII(nullptr) {}
>
> + /// Explicit conversion between forward/reverse iterators.
> + ///
> + /// Translate between forward and reverse iterators without changing range
> + /// boundaries. The resulting iterator will dereference (and have a handle)
> + /// to the previous node, which is somewhat unexpected; but converting the
> + /// two endpoints in a range will give the same range in reverse.
> + ///
> + /// This matches std::reverse_iterator conversions.
> + explicit MachineInstrBundleIterator(
> + const MachineInstrBundleIterator<Ty, !IsReverse> &I)
> + : MachineInstrBundleIterator(++I.getReverse()) {}
> +
> /// Get the bundle iterator for the given instruction's bundle.
> static MachineInstrBundleIterator getAtBundleBegin(instr_iterator MI) {
> return MachineInstrBundleIteratorHelper<IsReverse>::getBundleBegin(MI);
> @@ -258,6 +270,11 @@ public:
>
> nonconst_iterator getNonConstIterator() const { return MII.getNonConst(); }
>
> + /// Get a reverse iterator to the same node.
> + ///
> + /// Gives a reverse iterator that will dereference (and have a handle) to the
> + /// same node. Converting the endpoint iterators in a range will give a
> + /// different range; for range operations, use the explicit conversions.
> reverse_iterator getReverse() const { return MII.getReverse(); }
> };
>
>
> Modified: llvm/trunk/unittests/ADT/IListIteratorTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/IListIteratorTest.cpp?rev=294349&r1=294348&r2=294349&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/ADT/IListIteratorTest.cpp (original)
> +++ llvm/trunk/unittests/ADT/IListIteratorTest.cpp Tue Feb 7 15:03:50 2017
> @@ -131,4 +131,44 @@ TEST(IListIteratorTest, CheckEraseRevers
> EXPECT_EQ(L.rend(), RI);
> }
>
> +TEST(IListIteratorTest, ReverseConstructor) {
> + simple_ilist<Node> L;
> + const simple_ilist<Node> &CL = L;
> + Node A, B;
> + L.insert(L.end(), A);
> + L.insert(L.end(), B);
> +
> + // Save typing.
> + typedef simple_ilist<Node>::iterator iterator;
> + typedef simple_ilist<Node>::reverse_iterator reverse_iterator;
> + typedef simple_ilist<Node>::const_iterator const_iterator;
> + typedef simple_ilist<Node>::const_reverse_iterator const_reverse_iterator;
> +
> + // Check conversion values.
> + EXPECT_EQ(L.begin(), iterator(L.rend()));
> + EXPECT_EQ(++L.begin(), iterator(++L.rbegin()));
> + EXPECT_EQ(L.end(), iterator(L.rbegin()));
> + EXPECT_EQ(L.rbegin(), reverse_iterator(L.end()));
> + EXPECT_EQ(++L.rbegin(), reverse_iterator(++L.begin()));
> + EXPECT_EQ(L.rend(), reverse_iterator(L.begin()));
> +
> + // Check const iterator constructors.
> + EXPECT_EQ(CL.begin(), const_iterator(L.rend()));
> + EXPECT_EQ(CL.begin(), const_iterator(CL.rend()));
> + EXPECT_EQ(CL.rbegin(), const_reverse_iterator(L.end()));
> + EXPECT_EQ(CL.rbegin(), const_reverse_iterator(CL.end()));
> +
> + // Confirm lack of implicit conversions.
> + static_assert(std::is_convertible<iterator, reverse_iterator>::value,
> + "unexpected implicit conversion");
> + static_assert(std::is_convertible<reverse_iterator, iterator>::value,
> + "unexpected implicit conversion");
> + static_assert(
> + std::is_convertible<const_iterator, const_reverse_iterator>::value,
> + "unexpected implicit conversion");
> + static_assert(
> + std::is_convertible<const_reverse_iterator, const_iterator>::value,
> + "unexpected implicit conversion");
> +}
> +
> } // end namespace
>
> Modified: llvm/trunk/unittests/CodeGen/MachineInstrBundleIteratorTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/CodeGen/MachineInstrBundleIteratorTest.cpp?rev=294349&r1=294348&r2=294349&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/CodeGen/MachineInstrBundleIteratorTest.cpp (original)
> +++ llvm/trunk/unittests/CodeGen/MachineInstrBundleIteratorTest.cpp Tue Feb 7 15:03:50 2017
> @@ -130,4 +130,68 @@ TEST(MachineInstrBundleIteratorTest, Com
> ASSERT_TRUE(CI != CMBI.getIterator());
> }
>
> +struct MyUnbundledInstr
> + : ilist_node<MyUnbundledInstr, ilist_sentinel_tracking<true>> {
> + bool isBundledWithPred() const { return false; }
> + bool isBundledWithSucc() const { return false; }
> +};
> +typedef MachineInstrBundleIterator<MyUnbundledInstr> unbundled_iterator;
> +typedef MachineInstrBundleIterator<const MyUnbundledInstr>
> + const_unbundled_iterator;
> +typedef MachineInstrBundleIterator<MyUnbundledInstr, true>
> + reverse_unbundled_iterator;
> +typedef MachineInstrBundleIterator<const MyUnbundledInstr, true>
> + const_reverse_unbundled_iterator;
> +
> +TEST(MachineInstrBundleIteratorTest, ReverseConstructor) {
> + simple_ilist<MyUnbundledInstr, ilist_sentinel_tracking<true>> L;
> + const auto &CL = L;
> + MyUnbundledInstr A, B;
> + L.insert(L.end(), A);
> + L.insert(L.end(), B);
> +
> + // Save typing.
> + typedef MachineInstrBundleIterator<MyUnbundledInstr> iterator;
> + typedef MachineInstrBundleIterator<MyUnbundledInstr, true> reverse_iterator;
> + typedef MachineInstrBundleIterator<const MyUnbundledInstr> const_iterator;
> + typedef MachineInstrBundleIterator<const MyUnbundledInstr, true>
> + const_reverse_iterator;
> +
> + // Convert to bundle iterators.
> + auto begin = [&]() -> iterator { return L.begin(); };
> + auto end = [&]() -> iterator { return L.end(); };
> + auto rbegin = [&]() -> reverse_iterator { return L.rbegin(); };
> + auto rend = [&]() -> reverse_iterator { return L.rend(); };
> + auto cbegin = [&]() -> const_iterator { return CL.begin(); };
> + auto cend = [&]() -> const_iterator { return CL.end(); };
> + auto crbegin = [&]() -> const_reverse_iterator { return CL.rbegin(); };
> + auto crend = [&]() -> const_reverse_iterator { return CL.rend(); };
> +
> + // Check conversion values.
> + EXPECT_EQ(begin(), iterator(rend()));
> + EXPECT_EQ(++begin(), iterator(++rbegin()));
> + EXPECT_EQ(end(), iterator(rbegin()));
> + EXPECT_EQ(rbegin(), reverse_iterator(end()));
> + EXPECT_EQ(++rbegin(), reverse_iterator(++begin()));
> + EXPECT_EQ(rend(), reverse_iterator(begin()));
> +
> + // Check const iterator constructors.
> + EXPECT_EQ(cbegin(), const_iterator(rend()));
> + EXPECT_EQ(cbegin(), const_iterator(crend()));
> + EXPECT_EQ(crbegin(), const_reverse_iterator(end()));
> + EXPECT_EQ(crbegin(), const_reverse_iterator(cend()));
> +
> + // Confirm lack of implicit conversions.
> + static_assert(!std::is_convertible<iterator, reverse_iterator>::value,
> + "unexpected implicit conversion");
> + static_assert(!std::is_convertible<reverse_iterator, iterator>::value,
> + "unexpected implicit conversion");
> + static_assert(
> + !std::is_convertible<const_iterator, const_reverse_iterator>::value,
> + "unexpected implicit conversion");
> + static_assert(
> + !std::is_convertible<const_reverse_iterator, const_iterator>::value,
> + "unexpected implicit conversion");
> +}
> +
> } // end namespace
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list