Adopt ~Copyable in Subprocess
#38
Open
Conversation
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
iCharlesHu
force-pushed
the
charles/noncopyable-execution
branch
3 times, most recently
from
821723e to
2754c5f
Compare
iCharlesHu
force-pushed
the
charles/noncopyable-execution
branch
from
2754c5f to
f126559
Compare
| // Input | ||
| var result: Int32 = -1 | ||
| if let inputRead = inputPipe.readFileDescriptor { | ||
| result = posix_spawn_file_actions_adddup2(&fileActions, inputRead.platformDescriptor, 0) | ||
| if inputReadFileDescriptor != nil { |
There was a problem hiding this comment.
Suggest using if let inputReadFileDescriptor { so you don't need the force unwrap below.
There was a problem hiding this comment.
Unfortunately this pattern, along with if let a = a is not compatible with ~Copyable types. Writing if let inputReadFileDescriptor would have consumed inputReadFileDescriptor so the line below would be double consume.
parkera
reviewed
May 14, 2025
iCharlesHu
force-pushed
the
charles/noncopyable-execution
branch
from
f126559 to
2d28695
Compare
iCharlesHu
force-pushed
the
charles/noncopyable-execution
branch
from
2d28695 to
7c35edb
Compare
|
Addressed review comments and removed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR introduces
~CopyableintoSubprocess, includingCreatedPipe,TrackedFileDescriptor, andTrackedDispatchIO.Resolves #37
Resolves #2
Additionally, we opted to slightly modify the API by moving the
.standardOutputand.standardErrorAsyncSequencefromExecutionto the closure parameter, alongsideExecution. This adjustment eliminated the necessity forAtomic(andAtomicBox) withinExecution.Furthermore, this change rendered
SequenceOutputinternal, as we now rely on overloads to infer the output and error types.This patch also drops support for
Swift 6.0because it can't properly support the~Copyablework. NowSwift 6.1is the minimal required Swift version to buildSubprocess