[PATCH] D103948: [llvm] Make Sequence reverse-iterable

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: mlir/include/mlir/IR/BuiltinAttributes.td:710
     using iterator =
-        llvm::mapped_iterator<llvm::detail::value_sequence_iterator<ptrdiff_t>,
+        llvm::mapped_iterator<typename decltype(llvm::seq<ptrdiff_t>(0, 0))::const_iterator,
                               std::function<T(ptrdiff_t)>>;
----------------
Quuxplusone wrote:
> gchatelet wrote:
> > The purpose of this change was to prevent mlir from depending on implementation details of llvm.
> > 
> > @mehdi_amini I've added `typename` to disambiguate between value and type for GCC5 but I have no way of checking whether that works or not (GCC5 is not available on my machine).
> > Do you have any suggestions on how to test it?
> > 
> Indeed the error message looked to me like the symptom of "forgetting to include the right header somewhere," because the compiler seemed to be complaining that it didn't know what `ptrdiff_t` meant. Adding `typename` here also LGTM.
> Nit: `const_iterator` could be just `iterator`, right? There's no difference between const-iterating and regular-iterating an `llvm::seq`, so the difference between them is nonexistent and we might as well use the shorter simpler name.
I usually use a docker container to test this, but feel free to land it, we can always revert it if it breaks a bot again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103948