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

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet added inline comments.


================
Comment at: llvm/include/llvm/ADT/Sequence.h:18
 
-#include "llvm/ADT/iterator.h"
-#include "llvm/ADT/iterator_range.h"
-#include <algorithm>
+#include "llvm/ADT/SmallVector.h"
+
----------------
@Quuxplusone @mehdi_amini Introducing the `asSmallVector()` method add a dependency in `Sequence`.
I'm having a hard time evaluating the costs/benefits balance of this move.



================
Comment at: llvm/include/llvm/ADT/Sequence.h:182
+template <typename ValueT> auto seq(ValueT Begin, ValueT End) {
+  return iota_range<ValueT>(std::move(Begin), std::move(End));
 }
----------------
@Quuxplusone I will remove the `std::move` in this patch (cf. D103900)


================
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)>>;
----------------
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?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103948