InputMultispan and OutputMultispan + basic adoption in RigidArray and UniqueArray#579
InputMultispan and OutputMultispan + basic adoption in RigidArray and UniqueArray#579Catfish-Man wants to merge 38 commits intoapple:mainfrom
Conversation
Co-authored-by: Guillaume Lessard <glessard@tffenterprises.com>
| @safe | ||
| @frozen | ||
| @available(SwiftStdlib 5.0, *) | ||
| public struct OutputMultispan<Element: ~Copyable>: ~Copyable, ~Escapable { |
There was a problem hiding this comment.
Would be interesting to see if we could add a conversion method that vends you an io_vec from this safely
There was a problem hiding this comment.
ooh that would be nice
|
I'll take a look at any CI failures, the tests pass locally so they're probably just forgetting to update one of the other build systems |
Sources/ContainersPreview/Extensions/OutputMultispan+Extras.swift
Outdated
Show resolved
Hide resolved
|
|
||
| // Shift elements after the subrange if necessary | ||
| let shiftDistance = addedCount | ||
| if shiftDistance != 0 && subrange.upperBound < count { |
There was a problem hiding this comment.
It looks like this code accounts for the possibility of multiple inline elements, but line 145 above assumes the presence of only a single inline element. Should they be made consistent?
There was a problem hiding this comment.
I really want to generalize the rest of it to support multiple inline elements, but I haven't decided how practical that is. Picking one option definitely seems good though.
Sources/ContainersPreview/Extensions/OutputMultispan+Extras.swift
Outdated
Show resolved
Hide resolved
| #endif | ||
|
|
||
| @frozen @usableFromInline | ||
| internal struct MultispanBuffer<Element: ~Copyable> { |
There was a problem hiding this comment.
NIT: Do you want to keep these shared internal types in this file our should we move them into a separate file?
There was a problem hiding this comment.
I think if they grow any further functionality they should probably move, but for the moment I'm fine with leaving them there
|
Windows failures are a CI issue: failed to connect to the docker API at npipe:////./pipe/docker_engine; check if the path is correct and if the daemon is running: open //./pipe/docker_engine: The system cannot find the file specified. |
|
The Windows failures are there to gently remind us that we exist to serve CI, not the other way around -- rerunning the failing jobs usually makes them pass. ¯\_(ツ)_/¯ |
|
I added a commit to make the PR build in package form -- however, the tests are crashing right now. |
|
This looks unrelated to my change… |
|
aaaand trying to test locally crashes |
So helpful |
|
@Catfish-Man You will need to rebase your change to pull in #615 |
|
Android failure is Windows is Linux hashed containers is etc… Linux containers is etc… |
e9f8d83 to
2e1f550
Compare
This gets us the core "asynchronously create a unique/rigid array with a multi output span"