[PATCH] D136777: llvm::zip should not require assignment operator

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 27 15:22:29 PDT 2022


dblaikie added a comment.

In D136777#3890029 <https://reviews.llvm.org/D136777#3890029>, @mehdi_amini wrote:

>> I've got mixed feelings - the mechanics of the change seem fine, but iterator requirements include copy assignability and copy constructibility - so whatever iterator you're going to use with this feature isn't implementing the iterator requirements and probably should be fixed?
>
> It's a good point that we're supporting here iterators that aren't compliant with the general requirements. But I'm wondering if this isn't still a good thing?
>
> That is: isn't it just good for ranges and all adaptors/filters/... to require the minimum amount of things they need from an iterator? (it's not like we're checking on every properties of an iterator everywhere just to force an arbitrary contract).
>
> (we should also fix the iterator if possible, but I know also that some iterators are "fat" and avoiding copies may be valuable!)

Yeah, that's roughly my thinking on approving this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136777



More information about the llvm-commits mailing list