• Quuxplusone Mordante var-const
- Group Reviewers
- rG1e77b396ffe4: [libc++] Add ranges::in_fun_result
Here and modulemap, please alphabetize.
Could this plausibly use the MoveOnly type from test/support/moveonly.h instead, just to reduce the line count a bit?
|1 ↗||(On Diff #398743)|
Please fix the spelling of this file's filename, at some point (here or elsewhere or just as an NFC commit).
in_out_result isn't listed here; and in_in_out_result either isn't listed, or merge-conflicts.
Same as on the in_in_out review, I think A would benefit from a comment about // no implicit conversion, and B could just be B(int);
Does this file need the #pragma GCC system_header annotation?
Note: in in_out_result, I used more descriptive/verbose names like _InputIterator. Ideally these should be consistent. I think that having long names is more common in __algorithm/ (quick grepping shows ~360 hits for _InputIterator and 0 hits for _Ip in my current working copy).
in_out_result is at the bottom of synopsis (around line 655). I don't have a very strong opinion on where in the synopsis these should be listed, but they should all be together; I'd slightly prefer having them in the end because I feel that these are helper types that don't really need to be in the spotlight.
Please add return 0;.
Can you please move the corresponding tests in in_out_result.compile.pass.cpp to this file for consistency? (not necessarily in this patch)
Would it be worthwhile to also test for the case of Empty and Empty2 where both classes are empty but they are distinct types?
Optional: maybe move definitions of ConstructibleFrom and Empty down so that they're right above test() definition and thus closer to the point of usage?
Optional: consider adding a brief comment to each test case, e.g. Conversion via copy construction and Conversion via casting (also tests that move-only types can be converted).
Perhaps also test with a copy-only type? (that could be overkill)
Ultranit: please include <utility>.
@philnik thanks for moving in_out_result up! :)
This was marked "Done", but not done. Since you moved the synopsis comment for in_out_result, I think it would be nice to also move the mentioned tests in this same commit. But if it happens in a followup, also cool.