[PATCH] D78796: [Support] Refactor LEB128 encoding into an input iterator

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

In D78796#2025835 <https://reviews.llvm.org/D78796#2025835>, @nlguillemot wrote:

> In D78796#2025650 <https://reviews.llvm.org/D78796#2025650>, @dsanders wrote:
>
> > I'm inclined to agree that the patch series as-is doesn't really warrant the iterators as the interface as no callers have been updated. However, I also don't see much that's iterator specific (ULEBifier would be roughly similar code leaving the iterator portion as trivial wrappers on the ULEBifier) and there are a few places (particularly in tablegen) that are emitting LEB's into containers where the loop to add bytes one by one is just noise and something like `std::copy(to_uleb(...), std::back_inserter(Table));` would be somewhat nice. The loop could easily be hidden in something like `append_uleb(Table, ...)` though I don't think there's a strong argument for (or against) iterators.
> >
> > What I would suggest is separating out the byte-sequence generation into a ULEBifier as David suggested but still keeping the iterator object as a thin adapter (effectively implementing that while loop) to support inter-operation with STL functions like std::copy.
>
>
> What is the core issue with the iterator interface that makes it desirable to have something like `ULEBifier<T>` instead?


For me it's that iterators reference and indirect into the elements of a container. They shouldn't be the container themselves


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

https://reviews.llvm.org/D78796