dblaikie added a comment. In D67647#1674745 <https://reviews.llvm.org/D67647#1674745>, @comex wrote:
> Here's a new version of the patch that uses iterator ranges instead of > `ArrayRef`, to avoid adding new uses of `CallExpr::getArgs()` in case it has > to be removed in the future due to the strict aliasing issue. Not related to the strict aliasing issue - just seemed like it'd be tidier than reconstructing ranges manually from getArgs and getNumArgs. > To support this, the following additional changes are included: I wasn't expecting it to involve all this work - but instead to change "arguments()" to return ArrayRef. Though perhaps you didn't go that route to allow future fixes to the strict aliasing (by having an adapting iterator - or perhaps arguments() already uses such an iterator that doesn't hit the strict aliasing issue?) needing to do even more cleaunp work? > > > - Fix `CastIterator`'s use of `iterator_adaptor_base` so that `operator[]` > isn't broken. > - Add an `operator[]` implementation to `iterator_range`. I'm not sure that's appropriate - not all types with begin() and end() (even those with random access iterators) would have op[], so it doesn't seem appropriate to add it/depend on it? > - Improve the existing `llvm::drop_begin` helper to assert that it doesn't go > out of bounds. (The implementation would be a lot less ugly if we could use > C++17 features; oh well.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67647/new/ https://reviews.llvm.org/D67647 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
