riccibruno marked an inline comment as done. riccibruno added inline comments.
================ Comment at: include/clang/AST/Expr.h:5103 + using reference = AssociationTy<Const>; + using pointer = AssociationTy<Const>; + AssociationIteratorTy() = default; ---------------- aaron.ballman wrote: > Carrying over the conversation from D57098: > > >> @aaron.ballman Cute, but I suspect this may come back to bite us at some > >> point. For instance, if someone thinks they're working with a real > >> pointer, they're likely to expect pointer arithmetic to work when it won't > >> (at least they'll get compile errors though). > > @riccibruno Hmm, but pointer is just the return type of operator-> no ? Is > > it actually required to behave like a pointer ? The only requirement I can > > find is that It->x must be equivalent to (*It).x, which is true here. > > I double-checked and you're right, this is not a requirement of the STL. > > > Also looking at the requirements for forward iterators I think that this > > iterator should actually be downgraded to an input iterator, because of the > > requirement that reference = T&. > > My concern is that with the less functional iterators, common algorithms get > more expensive. For instance, `std::distance()`, `std::advance()` become more > expensive without random access, and things like `std::prev()` become > impossible. > > It seems like the view this needs to provide is a read-only access to the > `AssociationTy` objects (because we need to construct them on the fly), but > not the data contained within them. If the only thing you can get back from > the iterator is a const pointer/reference/value type, then we could store a > local "current association" object in the iterator and return a > pointer/reference to that. WDYT? I am worried about lifetime issues with this approach. Returning a reference/pointer to an `Association` object stored in the iterator means that the reference/pointer will dangle as soon as the iterator goes out of scope. This is potentially surprising since the "container" (that is the `GenericSelectionExpr`) here will still be in scope. Returning a value is safer in this aspect. I believe it should be possible to make the iterator a random access iterator, at least if we are willing to ignore some requirements: 1.) For forward iterators and up, we must have `reference = T&` or `const T&`. 2.) For forward iterators and up, `It1 == It2` if and only if `*It1` and `*It2` are bound to the same object. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57106/new/ https://reviews.llvm.org/D57106 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
