Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #238 +/- ##
==========================================
- Coverage 97.74% 96.85% -0.90%
==========================================
Files 146 148 +2
Lines 5149 5147 -2
Branches 244 301 +57
==========================================
- Hits 5033 4985 -48
- Misses 80 127 +47
+ Partials 36 35 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tcbrindle
force-pushed
the
devel
branch
3 times, most recently
from
b2eda01 to
f18a97d
Compare
...to `seq_for_each_while()`
Not quite all of them yet though
...and while we're at it, use the new std::from_range constructors in C++23 if we can, and the revised formulation for appending elements
Turns out it is needed sometimes
I looks like the very neat CTAD-via-designated-initialisers unfortunately doesn't work with Clang 17
...this one dealing with plain iterables
Injected class name causes chaos! This was a weird on to track down.
This adds `iterable` conformance to `generator` and removes `sequence`
Previously, if the user specified a non-reference `T` in `generator<T>`, we changed the actual element type to `const T&`, the reason being that it was possible to call `read_at()` multiple times so we couldn't safely yield an rvalue (reference). Under the new model, we can only read each value once, so it's safe to actually give the user what they ask for -- well, almost. Instead of yielding a prvalue we actually use an xvalue to save a move.
...but no longer sequences. Also, remove the `from_istreambuf()` function -- pass an lvalue istreambuf to `iterate()` instead.
...rather than plain old `element_t` and `value_t` in inline_sequence_base. Asking for one of the sequence typedefs with a non-sequence (i.e. just an iterable) can result in hard errors, which is really really annoying.
Maybe...
...which apparently turned up lots of other bugs
STL iterators are required to be at least movable. Since iteration contexts are typically non-copyable and non-movable, we cannot store one inside a valid iterator. This means that in order to use plain iterables with STL algorithms we need to use the `as_range` adaptor, which can hold the iter_ctx and hand out movable iterators which hold a pointer back to it. However, range-for loops have considerably weaker requirements than the standard library concepts. So to enable iterables to be directly used with range-for loops, this commit adds "pseudoiterators" which meet the bare minimum requirements that the range-for needs. These pseudoiterators won't work with standard library algorithms, and in fact we deliberately don't add extra typedefs and things just to try to emphasise that they're not real iterators.
Instead, use flux::ref(), flux::mut_ref() or flux::from(). Under the new model, all ranges are at least iterable. Sized, random-access ranges will also become collections. However, the new collection requirements will mean that we will no longer be able to treat non-RA ranges as collections (even if we were willing to do so unsafely).
Whereas the translation to iterables has simplified most adaptors, these seem a bit tricky to move to the new API, as they require comparing the same element multiple times (and therefore internal caching). For now we'll just require their bases to be multipass sequences.
Use std::span instead (begrudgingly)
For an incrementable type, iota(i) gives an infinite iterable, and iota(i, j) gives an iterable that will iterate until i == j. If the type is additionally decrementable, then iota(i, j) will additionally be reverse iterable. If the type is additionally advanceable, then iota(i, j) will also be sized. If the type is strongly ordered (with op<=>) then it is a multipass sequence (and in future, a collection), plus bidir if decrementable and RA if advancable. Finally, for a type which is `std::is_arithmetic_v<T>`, iota(a) is equivalent to `iota(a, numeric_limits<T>::max())` -- that is, we treat it as a bounded collection rather than an infinite iterable.
This is an interesting one, because we're actually removing functionality -- infinite repeat was previously random-access, but we can't do that when we move to collections. It does raise the question of whether impls should be able to override (certain? all?) algorithms -- because now `repeat(x).take(n)` is iterable-only, whereas we want it to work the same as `repeat(x, n)`, i.e. be random-access. Also, repeat(x).drop(n) could be a no-op, and stride() and reverse() and...
That was easy
It's causing odd linker errors relating to flux::detail::unreachable, not really sure what the problem is
Apparently consistency errors with code inlining lambdas is a known problem
The whole point of it is that it never gets called...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.