[PATCH] D67647: [Consumed] Refactor handleCall to take function argument list. NFC.

Wed Sep 18 17:38:45 PDT 2019

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