Implement code review feedback, with some minor tweaks because initializer list isn't convertable, and the iterator doesn't have the traits for std::next

Author: Jlalond

llvm/include/llvm/Object/Minidump.h

@@ -221,27 +221,20 @@ class MinidumpFile : public Binary {
 
   class ExceptionStreamsIterator {
   public:
-    static ExceptionStreamsIterator begin(ArrayRef<minidump::Directory> Streams,
-                                          const MinidumpFile *File) {
-      return ExceptionStreamsIterator(Streams, File);
-    }
-
-    static ExceptionStreamsIterator end() { return ExceptionStreamsIterator(); }
+    ExceptionStreamsIterator(ArrayRef<minidump::Directory> Streams,
+                            const MinidumpFile *File)
+      : Streams(Streams), File(File) {}
 
     bool operator==(const ExceptionStreamsIterator &R) const {
-      return Streams.empty() && R.Streams.empty();
+      return Streams.size() == R.Streams.size();
     }
 
     bool operator!=(const ExceptionStreamsIterator &R) const {
       return !(*this == R);
     }
 
     Expected<const minidump::ExceptionStream &> operator*() {
-      return ReadCurrent();
-    }
-
-    Expected<const minidump::ExceptionStream &> operator->() {
-      return ReadCurrent();
+      return File->getExceptionStream(Streams.front());
     }
 
     ExceptionStreamsIterator &operator++() {
@@ -252,25 +245,8 @@ class MinidumpFile : public Binary {
     }
 
   private:
-    ExceptionStreamsIterator(ArrayRef<minidump::Directory> Streams,
-                             const MinidumpFile *File)
-        : Streams(Streams), File(File) {}
-
-    ExceptionStreamsIterator()
-        : Streams(ArrayRef<minidump::Directory>()), File(nullptr) {}
-
     ArrayRef<minidump::Directory> Streams;
     const MinidumpFile *File;
-
-    Expected<const minidump::ExceptionStream &> ReadCurrent() {
-      assert(!Streams.empty());
-      Expected<const minidump::ExceptionStream &> ExceptionStream =
-          File->getExceptionStream(Streams.front());
-      if (!ExceptionStream)
-        return ExceptionStream.takeError();
-
-      return ExceptionStream;
-    }
   };
 
   using FallibleMemory64Iterator = llvm::fallible_iterator<Memory64Iterator>;

llvm/lib/Object/Minidump.cpp

@@ -55,8 +55,8 @@ Expected<std::string> MinidumpFile::getString(size_t Offset) const {
 
 iterator_range<llvm::object::MinidumpFile::ExceptionStreamsIterator>
 MinidumpFile::getExceptionStreams() const {
-  return make_range(ExceptionStreamsIterator::begin(ExceptionStreams, this),
-                    ExceptionStreamsIterator::end());
+  return make_range(ExceptionStreamsIterator(ExceptionStreams, this),
+                    ExceptionStreamsIterator({}, this));
 }
 
 Expected<iterator_range<MinidumpFile::MemoryInfoIterator>>
@@ -167,7 +167,7 @@ MinidumpFile::create(MemoryBufferRef Source) {
   }
 
   return std::unique_ptr<MinidumpFile>(new MinidumpFile(
-      Source, Hdr, *ExpectedStreams, std::move(StreamMap), ExceptionStreams));
+      Source, Hdr, *ExpectedStreams, std::move(StreamMap), std::move(ExceptionStreams)));
 }
 
 iterator_range<MinidumpFile::FallibleMemory64Iterator>

llvm/unittests/Object/MinidumpTest.cpp

@@ -711,7 +711,7 @@ TEST(MinidumpFile, getMemoryInfoList) {
                                    0x0001000908000000u));
 }
 
-TEST(MinidumpFile, getExceptionStream) {
+TEST(MinidumpFile, getExceptionStreams) {
   std::vector<uint8_t> Data{
       // Header
       'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
@@ -751,8 +751,10 @@ TEST(MinidumpFile, getExceptionStream) {
   auto ExpectedFile = create(Data);
   ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
   const MinidumpFile &File = **ExpectedFile;
-  auto ExceptionIterator = File.getExceptionStreams().begin();
-
+  
+  auto ExceptionStreams = File.getExceptionStreams();
+  ASSERT_NE(ExceptionStreams.begin(), ExceptionStreams.end());
+  auto ExceptionIterator = ExceptionStreams.begin();
   Expected<const ExceptionStream &> ExpectedStream = *ExceptionIterator;
   ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
   EXPECT_EQ(0x04030201u, ExpectedStream->ThreadId);
@@ -768,4 +770,6 @@ TEST(MinidumpFile, getExceptionStream) {
   }
   EXPECT_EQ(0x84838281, ExpectedStream->ThreadContext.DataSize);
   EXPECT_EQ(0x88878685, ExpectedStream->ThreadContext.RVA);
+  ++ExceptionIterator;
+  ASSERT_EQ(ExceptionIterator, ExceptionStreams.end());
 }