chunk_by_view's helper lambda returns bool - #2890
Conversation
Michael Schellenberger Costa (miscco)
left a comment
There was a problem hiding this comment.
We are missing some additional test coverage.
ef4326b to
8a13123
Compare
nicole mazzuca (strega-nil-ms)
left a comment
There was a problem hiding this comment.
Removing my approval as I do not believe it correctly implements the standard.
nicole mazzuca (strega-nil-ms)
left a comment
There was a problem hiding this comment.
It definitely took a bit to convince me that this is correct, but I have now been convinced.
Casey Carter (CaseyCarter)
left a comment
There was a problem hiding this comment.
Small suggestions, of which we have enough that I think I should push changes.
| #else // ^^^ no workaround / workaround vvv | ||
| indirect_result_t<_Proj&, _It> operator*() const { | ||
| _CSTD abort(); // TRANSITION, VSO-1308657 | ||
| _CSTD abort(); |
There was a problem hiding this comment.
For reviewers:
projected is a tool to simplify expressing the requirements of algorithms that use projections. The design intent is that the member functions don't have definitions - they are just signatures that exist for iter_reference_t et al. to look at. This hasn't worked well in practice in the real world, as witnessed by the warning suppressions for clang and MSVC around the definition, the additional bug report (VSO-1308657) mentioned there, and the fact that it broke yet again when I made Bool a local class in that PR. The PR uses projected<Bool*, identity>, and clang-cl decided to report an error about operator* being used-but-not-defined when it can't possibly be defined in another TU since Bool has internal linkage. Rather than trying to find yet another workaround for this perennial issue (four or five changes here in ~3 years?) I decided to give up and simply always define operator* to call abort. The function should never be called in practice, so even if it does end up emitted into some object files the linker should be able to clean it up.
nicole mazzuca (strega-nil-ms)
left a comment
There was a problem hiding this comment.
I am sad about projected::operator*, but cools.
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
|
Thanks for fixing this compiler error and improving the ranges experience! 😻 🐞 🎉 |
Co-authored-by: Casey Carter <Casey@Carter.net>
Co-authored-by: Casey Carter <Casey@Carter.net>
Fixes #2889
Casey's drive-by: Give up on the Standard's dream that we can avoid defining
projected::operator*. There are just too many interactions with the "need for constant evaluation" rules and the use of types with internal linkage - it's not worth fighting anymore.