[libcxx-commits] [PATCH] D109086: [libc++] Implement ranges::filter_view

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__ranges/filter_view.h:124-125
+    using iterator_category =
+      _If<derived_from<bidirectional_iterator_tag, _Cat>, bidirectional_iterator_tag,
+      _If<derived_from<forward_iterator_tag, _Cat>,       forward_iterator_tag,
+      /* else */                                          _Cat
----------------
These conditions smell backwards. Shouldn't it be `_Cat satisfies derived_from<foo_tag>`, i.e. `derived_from<_Cat, foo_tag>`?
But actually I would suggest testing the full concept: `_If<bidirectional_iterator<_Cat>, bidirectional_iterator_tag, ...`. Because (sadly) it's perfectly possible in C++20 to have an iterator advertising `bidirectional_iterator_tag` that itself is only a `forward_iterator`. The `forward_iterator` concept does not have any "penalty for overpromising."

Both the bug(?) and the sad scenario could use a test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109086