[llvm] r294349 - ADT: Add explicit conversions for reverse ilist iterators

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 7 13:42:54 PST 2017


Seeing some failures - missed negation I guess so I added some '!'
in r294357.

IListIteratorTest.cpp:162:3: error: static_assert failed "unexpected
implicit conversion"
  static_assert(std::is_convertible<iterator, reverse_iterator>::value,
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
IListIteratorTest.cpp:164:3: error: static_assert failed "unexpected
implicit conversion"
  static_assert(std::is_convertible<reverse_iterator, iterator>::value,
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
IListIteratorTest.cpp:166:3: error: static_assert failed "unexpected
implicit conversion"
  static_assert(
  ^
IListIteratorTest.cpp:169:3: error: static_assert failed "unexpected
implicit conversion"
  static_assert(
  ^

On Tue, Feb 7, 2017 at 1:15 PM 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170207/0a5ce215/attachment.html>


More information about the llvm-commits mailing list