[libc-commits] [PATCH] D121656: [libc][BlockStore] Add back, pop_back and empty methods

Tue Mar 15 08:15:41 PDT 2022

abrachet marked an inline comment as done.
abrachet added inline comments.

================
Comment at: libc/src/__support/CPP/blockstore.h:117
+  T &back() {
+    static_assert(REVERSE_ORDER);
+    return *begin();
----------------
sivachandra wrote:
> abrachet wrote:
> > sivachandra wrote:
> > > sivachandra wrote:
> > > > `REVERSE_ORDER` only controls the iteration. IMO, `back` and `pop_back` are independent of that and should work irrespective of the `REVERSE_ORDER`.
> > > I just realized I pressed submit while I was still thinking about this. Go ahead and submit this patch with one change: Instead of a `static_assert` in the function body, use a `EnableIf` for the return type. I have few other thoughts. They would be best expressed as a separate patch, which I will do after you land this.
> > Ah, didn't see this comment while working on it. How does this look? I can revert to revision 1 if you'd like. But I guess this is probably better
> Still fine. I don't quite like the `for` loop on line 56, but fine for now.
Agreed, this is why I initially had it only for `REVERSE_ORDER`. In any case there are no current users of BlockStore<T, N, false>

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121656