[libcxx-commits] [PATCH] D117512: [libc++][ranges] Add ranges::in_in_out_result

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/test/std/algorithms/algorithms.results/in_in_out_result.pass.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
Quuxplusone wrote:
> Quuxplusone wrote:
> > I don't see a CTAD test. How are we doing CTAD tests for these types? `deduct.pass.cpp`, or dump it in here, or what?
> This is still unaddressed, right?
> I think it would suffice to make an `algorithms.results/deduct.compile.pass.cpp` covering all the types, on the model of your existing `no_unique_address.compile.pass.cpp`.
> ```
> ASSERT_SAME_TYPE(decltype(std::ranges::in_in_result(1, 'x')), std::ranges::in_in_result<int, char>);
> ASSERT_SAME_TYPE(decltype(std::ranges::in_in_out_result(1, 'x', 1.f)), std::ranges::in_in_result<int, char, float>);
> ```
> etc.
There are no deduction guides for the result types. I don't think this is supposed to work.


================
Comment at: libcxx/test/std/algorithms/algorithms.results/in_in_out_result.pass.cpp:79
+    assert(res.out == 1.f);
+    std::ranges::in_in_out_result<ConstructibleFrom<int>, ConstructibleFrom<double>, ConstructibleFrom<float>> res2 = res;
+    assert(res2.in1.content == 10);
----------------
Quuxplusone wrote:
> philnik wrote:
> > Quuxplusone wrote:
> > > I don't think this buys you anything over a simple
> > > ```
> > > std::ranges::in_in_out_result<long, float, int> res2 = res;
> > > assert(res2.in1 == 10L);
> > > ~~~
> > > ```
> > > (Also, moot point now, but `ConstructibleFrom` would be the wrong name for something that's implicitly //convertible// from; `ConvertibleFrom` would have been a better name.)
> > It's the same as in D116278.
> Ah; consider removing it from both places here or in a followup. :)
I meant that the implicit conversion does not work. Your example will not compile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117512