Compilation error when using operator++ (post increment) function of SmallPtrSetIterator class

[Abhilash]

Summary

The function operator++ (post increment) in the SmallPtrSetIterator class has been recently changed to handle reverse iterations through command line option. The new change leads to a compilation error when operator++ is used.

The compilation error is:
/mySandBox/llvm/include/llvm/ADT/SmallPtrSet.h: In instantiation of 'llvm::SmallPtrSetIterator<PtrTy> llvm::SmallPtrSetIterator<PtrTy>::operator++(int) [with PtrTy = llvm::User*]':
/mySandBox/llvm/lib/Transforms/Scalar/myopt.cpp:418:32: required from here
/mySandBox/llvm/include/llvm/ADT/SmallPtrSet.h:295:7: error: no match for 'operator--' (operand type is 'llvm::SmallPtrSetIterator<llvm::User*>')

--*this;
^

make[2]: * [lib/Transforms/Scalar/CMakeFiles/LLVMScalarOpts.dir/myopt.cpp.o] Error 1
make[1]: * [lib/Transforms/Scalar/CMakeFiles/LLVMScalarOpts.dir/all] Error 2
make: *** [all] Error 2

The compilation fails stating that operator-- is not defined for SmallPtrSetIterator.
I see that, operator++ (post-increment) function internally calls operator++ (pre-increment). Since pre-increment already handles reverse iteration, I feel we do not need to specifically handle reverse iterations in the post-increment function.
This patch contains the corresponding changes.

The differential for the early changes: https://reviews.llvm.org/D26718

[mehdi_amini]

Can you update the unit test?

[Abhilash]

ReverseIterationTest has been updated.

[mehdi_amini]

LGTM. Thanks!

llvm/trunk/include/llvm/ADT/SmallPtrSet.h

 #endif
     ++Bucket;
     AdvanceIfNotValid();
     return *this;
   }
 
   SmallPtrSetIterator operator++(int) {        // Postincrement
     SmallPtrSetIterator tmp = *this;
-#if LLVM_ENABLE_ABI_BREAKING_CHECKS
-    if (ReverseIterate<bool>::value) {
-      --*this;
-      return tmp;
-    }
-#endif
     ++*this;
     return tmp;
   }
 };
 
 /// RoundUpToPowerOfTwo - This is a helper template that rounds N up to the next
 /// power of two (which means N itself if N is already a power of two).
 template<unsigned N>

llvm/trunk/unittests/ADT/ReverseIterationTest.cpp

TEST(ReverseIterationTest, SmallPtrSetTest) {
   for (auto *Ptr: Ptrs)
     Set.insert(Ptr);
 
   // Check forward iteration.
   ReverseIterate<bool>::value = false;
   for (const auto &Tuple : zip(Set, Ptrs))
     ASSERT_EQ(std::get<0>(Tuple), std::get<1>(Tuple));
 
+  // Check operator++ (post-increment) in forward iteration.
+  int i = 0;
+  for (auto begin = Set.begin(), end = Set.end();
+             begin != end; i++)
+    ASSERT_EQ(*begin++, Ptrs[i]);
+
   // Check reverse iteration.
   ReverseIterate<bool>::value = true;
   for (const auto &Tuple : zip(Set, ReversePtrs))
     ASSERT_EQ(std::get<0>(Tuple), std::get<1>(Tuple));
+
+  // Check operator++ (post-increment) in reverse iteration. 
+  i = 0;
+  for (auto begin = Set.begin(), end = Set.end();
+             begin != end; i++)
+    ASSERT_EQ(*begin++, ReversePtrs[i]);
+
 }
 #endif