Implement LWG-3392 ranges::distance() cannot be used on a move-only iterator with a sized sentinel
#2421
Conversation
Cleanup conditional `noexcept` while we're here, and add test coverage. Fixes microsoft#2392
StephanTLavavej
changed the title
ranges::distance() cannot be used on a move-only iterator with a sized sentinel
This comment has been minimized.
This comment has been minimized.
| enum class sized { no, yes }; | ||
| enum class assign { no, yes }; | ||
| enum class nothrow { no, yes }; |
There was a problem hiding this comment.
Why must these be enum classes instead of simple bools?
It is fun to read nothrow NoThrow = nothrow::no (no, NoThrow!) but noexcept(IsNoThrow) might be clearer...
Pre-existing
There was a problem hiding this comment.
Because it is much more difficult to reason about what meow<true> does compared to meow<nothrow::no>
Also it is much harder to accidentally pass the wrong thing
There was a problem hiding this comment.
Especially in this case where it is
trace_iterator<Cat, true, false, true>Are you sure those are in the right order?
There was a problem hiding this comment.
nothrow is a terrible name. How many times have I told someone not to put a negative in a name?
There was a problem hiding this comment.
But you told them not to do it.
Nobody said you could not commit horrible code crimes
There was a problem hiding this comment.
May I suggest something giving the best of both worlds (in the future, this is pre-existing, also using enum helps a ton):
enum : bool { Throws, NoThrow };
template<bool IsNoThrow = false>
struct meow { ... };
meow<NoThrow> ....There was a problem hiding this comment.
I suppose you could name the enum if you really want to, but that isn't relevant for clarity, just for preventing type confusion, but type confusion is arguably not a big deal here since really the type is bool for all of them.
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
|
Thanks again for implementing all of these ranges LWG issues! 😻 🚀 🪐 |
Cleanup conditional
noexceptwhile we're here, and add test coverage.Fixes #2392