Commit 0655e6d8 authored by Hans Wennborg's avatar Hans Wennborg
Browse files

Merging r294349 and r294357: 

------------------------------------------------------------------------
r294349 | dexonsmith | 2017-02-07 13:03:50 -0800 (Tue, 07 Feb 2017) | 12 lines

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.
------------------------------------------------------------------------

------------------------------------------------------------------------
r294357 | dblaikie | 2017-02-07 13:31:03 -0800 (Tue, 07 Feb 2017) | 1 line

Fix some missing negations in the traits checking from r294349
------------------------------------------------------------------------

llvm-svn: 294477
parent f3b5cd16

llvm/include/llvm/ADT/ilist_iterator.h

+16 −3
Original line number Diff line number Diff line
@@ -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);

llvm/include/llvm/CodeGen/MachineInstrBundleIterator.h

+17 −0
Original line number Diff line number Diff line
@@ -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(); }

};

llvm/unittests/ADT/IListIteratorTest.cpp

+40 −0
Original line number Diff line number Diff line
@@ -131,4 +131,44 @@ TEST(IListIteratorTest, CheckEraseReverse) { 
  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

llvm/unittests/CodeGen/MachineInstrBundleIteratorTest.cpp

+64 −0
Original line number Diff line number Diff line
@@ -130,4 +130,68 @@ TEST(MachineInstrBundleIteratorTest, CompareToBundledMI) { 
  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