[PATCH] D22079: Refactor Archive-child iteration.

[Lang Hames via llvm-commits]

lhames added a comment.

> OK, so the nullptr case is just for the end iterator? If so, can you

>  try to change the constructors to...

> 

>   private:

>   

>   child_iterator() : C(Child(nullptr, nullptr, nullptr)), E(nullptr) {}

>   public:

>   

>   child_iterator(const Child &C, Error &E) : C(C), E(&E) {}


I can switch the Error* parameter to an Error&, but I think the default constructor should still be public - iterators are usually default constructible.

> Can you change this to return Optional<Archive::Child>?

> 

> The iterator was only use a convenient form of Optional, but now that it requires an Error it is not as convenient for this function.


That would leave clients unable to detect the difference between a broken archive and an undefined symbol. I don't think we want to do that.

Cheers,
Lang.


Repository:
  rL LLVM

http://reviews.llvm.org/D22079