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

[Nicolas Guillemot via Phabricator via llvm-commits]

nlguillemot added a comment.

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?

If it's a matter of user interface design, then I would rather implement `ULEBifier<T>` in terms of the iterator, since the iterator is more generic.

If it's a matter of separating the iterator-specific boilerplate from the logic, then I would rather do that by refactoring the implementation to put the core logic in private member functions. That would make it more clear to see what is iterator boilerplate and what is not.

In either case, I would rather not have 2 APIs that do the same thing, for the sake of consistency in the codebase and to avoid code duplication.


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

https://reviews.llvm.org/D78796