[PATCH] D48348: [ADT] Add zip_longest iterators.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

In D48348#1310674 <https://reviews.llvm.org/D48348#1310674>, @dblaikie wrote:

> I /think/ this still doesn't meet the iterator requirements (that if "(*i).j" is valid, then "i->j" must be valid too - [input.iterators]p2, Table 95, "a->m is equivalent to (*a).m if a is dereferencable"). I believe the way standard iterators (like istream_iterator) implement this is by having an instance of the value type inside the iterator that has the current value in it, so operator-> can return a pointer to that. Would that be possible/practical?


The type of `*a` is a `std::tuple` of the client-iterator's value types. Storing an internal scratch field inside the the zip-operator would allow `operator->` to return a pointer. However, `std::tuple` has just two methods that could be called: `swap` and `operator=`. Both would be useless and `operator->` should probably return a `const std::tuple*` to avoid messing with the internal field anyway. It still would not allow assigning to the tuple's elements.

Experimenting with `llvm::zip` and `llvm::map_iterator` shows that they don't meet the requirement either:

  const SmallVector<unsigned, 6> pi{3, 1, 4, 1, 5, 9};
  auto i = map_iterator(pi.begin(), [](unsigned v) -> std::pair<unsigned,char> {return (v == 3) ? std::make_pair(v,'a') : std::make_pair(v,'\0');} );
  (*i).first;
  i->first;
  
  error: taking the address of a temporary object of type 'std::pair<unsigned int, char>' [-Waddress-of-temporary]
    PointerT operator->() { return &static_cast<DerivedT *>(this)->operator*(); }
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~



        SmallVector<unsigned, 6> pi{3, 1, 4, 1, 5, 9};
        auto i = zip(pi,pi).begin();
        std::tuple<const unsigned&,const unsigned&> x{pi[0],pi[1]};
        (*i).operator=(x);
        i->operator=(x);
  
  llvm/ADT/iterator.h:170:34: error: taking the address of a temporary object of type 'llvm::detail::zip_common<llvm::detail::zip_shortest<unsigned int *, unsigned int *>, unsigned int *, unsigned int *>::value_type' (aka 'std::tuple<unsigned int &, unsigned int &>') [-Waddress-of-temporary]
    PointerT operator->() { return &static_cast<DerivedT *>(this)->operator*(); }
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

To answer the question. Yes, it it possible to add a scratch `std::tuple` to `zip_longest` iterators, but there is no practical gain.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D48348/new/

https://reviews.llvm.org/D48348