Skip to content

Platform: make MSVCRT more Unix-libc like #21293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 14, 2018

Conversation

compnerd
Copy link
Member

Since stdin, stdout, and stderr are defined via macros, we cannot see them
on the swift side. Replicate these by hand. Expose STDIN_FILENO,
STDOUT_FILENO, and STDERR_FILENO for compatibility with other libc
implementations. This reduces the need for changing the codebase unnecessarily
for MSVCRT.

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

@compnerd
Copy link
Member Author

CC: @jrose-apple

@compnerd
Copy link
Member Author

@swift-ci please test

@jrose-apple
Copy link
Contributor

I feel like @moiseev or @lancep would be better at evaluating whether this is the way to go, since they've worked on the Darwin overlays.

@compnerd
Copy link
Member Author

I realized that I can play some indirection tricks and reduce the amount of code that needs to be effectively "if-defed", and this also makes user code be similar across platforms, so I think that this is an improvement.

// Unfortunately, the clang importer does not import "complex" macros such as
// the one that are used in the UCRT for defining `stdin`, `stdout`, `stderr`.
// Replicate them manually.
public var stdin: UnsafeMutablePointer<FILE> { return __acrt_iob_func(0) }
Copy link
Contributor

@moiseev moiseev Dec 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These variables are already defined in Platform.swift under a guard. Wouldn't it be better to have them in the same place?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sigh I had forgotten about that. Thanks! Yes, these belong there.

Since `stdin`, `stdout`, and `stderr` are defined via macros, we cannot see them
on the swift side.  Replicate these by hand.  Expose `STDIN_FILENO`,
`STDOUT_FILENO`, and `STDERR_FILENO` for compatibility with other libc
implementations.  This reduces the need for changing the codebase unnecessarily
for MSVCRT.
@compnerd
Copy link
Member Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 09f299db7f716fb8b1730c1f02c37f1bab6b3dd3

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 09f299db7f716fb8b1730c1f02c37f1bab6b3dd3

@compnerd compnerd merged commit ca0a622 into swiftlang:master Dec 14, 2018
@compnerd compnerd deleted the msvcrt-fileio branch December 14, 2018 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants