[libcxx-commits] [PATCH] D142302: [libc++][format] Fixes usage of contiguous ranges.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked an inline comment as done.
Mordante added a subscriber: ldionne.
Mordante added a comment.

Thanks for the reviews!

In D142302#4071698 <https://reviews.llvm.org/D142302#4071698>, @philnik wrote:

> We have iterators of all categories (and sized/unsized sentinels) in `test_iterators.h`. Why don't you use them?

In the tests for sets and maps @ldionne suggested to use adaptor classes with a minimal interface in the tests. So I thought it would be good to do the same here. I already use some the test iterators, just prior to the new hunk.



================
Comment at: libcxx/include/__format/range_formatter.h:175
+              ranges::data(__range),
+              ranges::size(__range),
+          },
----------------
JMazurkiewicz wrote:
> How about using `ranges::distance` (http://eel.is/c++draft/range.iter.op.distance#4) here instead of `ranges::size`? We could get rid of `ranges::sized_range<_Rp>` constraint.
> 
> Also, I think we need extra includes: `<__ranges/data.h>` and `<__ranges/size.h>` (or `<__iterator/distance.h>`).
I expected `ranges::distance` to require a `sized_range`, but I didn't check it. But you're right distance would work too and have less requirements.

Indeed these headers should be included. I'm surprised it passes the modular tests without them. 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142302