[libcxx-commits] [PATCH] D142335: [libc++][ranges] Implement `ranges::to`.

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const marked 13 inline comments as done.
var-const added inline comments.


================
Comment at: libcxx/include/__ranges/to.h:220-221
+      !is_volatile_v<_Container>, "The target container cannot be volatile-qualified, please remove the volatile");
+
+  return __range_adaptor_closure_t(std::__bind_back(__to_nondeduced<_Container>(), std::forward<_Args>(__args)...));
+}
----------------
ldionne wrote:
> Would it be reasonable to try using a lambda here? Something like
> 
> ```
> auto __to_func = []<input_range _Range, class... _Args>(_Range&& __range, _Args&&... __args) -> _Container 
>  requires requires { ranges::to<_Container>(std::forward<_Range>(__range), std::forward<_Args>(__args)...) }
> { return ranges::to<_Container>(std::forward<_Range>(__range), std::forward<_Args>(__args)...); };
> 
> return __range_adaptor_closure_t(std::__bind_back(__to_func, std::forward<_Args>(__args)...));
> ```
> 
> It's more compact and a bit more localized. Your call.
I tried it out and I think I like it better (though it looks like `clang-format` has trouble with a constrained lambda).


================
Comment at: libcxx/include/__ranges/to.h:67-69
+template <class _Container, class _Range>
+concept __can_try_convert_without_recursion =  !input_range<_Container> ||
+    convertible_to<range_reference_t<_Range>, range_value_t<_Container>>;
----------------
ldionne wrote:
> var-const wrote:
> > ldionne wrote:
> > > Maybe `__does_not_require_recursive_conversion`? Or `__try_non_recursive_conversion`? Or maybe just inline that in the function below and use an inline `requires`?
> > Went with `__try_non_recursive_conversion` (unless you feel strongly about inlining it).
> Re-reading it, I think it would be a lot more readable to inline, especially since there's an `else if` with the `input_range<_Container>` constraint.
Making this a concept is necessary to "short-circuit" the second condition (e.g. an `optional` satisfies the first condition, i.e., `!input_range`, but attempting to instantiate the second condition would fail since `range_value_t<optional<T>>` is ill-formed). Added a comment to that effect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142335