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

[Hui via Phabricator via libcxx-commits]

huixie90 added inline comments.


================
Comment at: libcxx/include/__format/range_formatter.h:175
+              ranges::data(__range),
+              ranges::size(__range),
+          },
----------------
Mordante wrote:
> 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. 
> 
> 
I disagree. Although it is really rare to see a `contiguous_range` that is not `sized_range`. But when this happens, I think we do want to constrain it to `sized_range`, otherwise `ranges::distance` is going to be a linear operation.  so using `ranges::size` is the right thing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142302