[PATCH] D103900: [llvm] Add enum iteration to Sequence
Guillaume Chatelet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 23 01:46:37 PDT 2021
gchatelet marked an inline comment as done.
gchatelet added inline comments.
================
Comment at: llvm/include/llvm/ADT/Sequence.h:28
-template <typename T, bool IsReversed> struct iota_range_iterator {
+template <typename U, typename T, bool IsReversed> struct iota_range_iterator {
using iterator_category = std::random_access_iterator_tag;
----------------
Quuxplusone wrote:
> Nits: Please add a newline before `struct`; and consider renaming/reordering the parameters so that `T` comes before `U`. I think that should be easy and mechanical (if tedious), because neither `T` nor `U` is deduced nor defaulted.
> Nits: Please add a newline before `struct`;
clang-format puts it back to the same line :-/
> and consider renaming/reordering the parameters so that `T` comes before `U`. I think that should be easy and mechanical (if tedious), because neither `T` nor `U` is deduced nor defaulted.
Done.
================
Comment at: llvm/include/llvm/ADT/Sequence.h:196
+ auto rbegin() const { return const_reverse_iterator(PastEndValue - 1); }
+ auto rend() const { return const_reverse_iterator(BeginValue - 1); }
----------------
Quuxplusone wrote:
> Note that `BeginValue-1` might be UB; e.g. `seq(INT_MIN, 0).rend()`. But I think it's fine to ignore this corner case.
> Note that `BeginValue-1` might be UB; e.g. `seq(INT_MIN, 0).rend()`. But I think it's fine to ignore this corner case.
Fixed it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103900/new/
https://reviews.llvm.org/D103900
More information about the llvm-commits
mailing list