Platform: port to msvcrt, add msvcrt module #3428
Conversation
|
CC: @gribozavr |
|
What made you decide on these particular names? ("msvcrt", "ucrt") |
|
msvcrt: thats what its always been called (Microsoft Visual C Runtime). |
|
I guess more specifically: why msvcrt instead of, e.g., MSVCRT? Is there any convention from the managed runtimes we can follow? (It's not an unreasonable convention for module names to follow DLL names on Windows. On the other hand, if we think Clang will ever start defining module maps for Windows, we probably want to use the same naming conventions so that switching over is less painful.) |
|
Im not aware of any managed runtimes mapping those libraries specifically. I just followed the library names. Im not tied to the name if there is a better solution. I don't think that we can really ever ship module maps for msvcrt in clang. Microsoft could (and has) altered headers associated with the library. |
|
AIUI, |
|
They are the one and the same. msvcrt is the c library. These days, its actually versioned (e.g. msvcr140.dll). msvcrt refers to that entity. You link against msvcrt.lib which is the import library for the dll. |
|
@swift-ci Please test |
|
Okay, if Dmitri's happy with the implementation I'm good with the names. |
| @@ -187,6 +209,7 @@ public func open( | |||
| return _swift_Platform_open(path, oflag, 0) | |||
| } | |||
|
|
|||
| #if !os(Windows) || CYGWIN | |||
| public func open( | |||
| _ path: UnsafePointer<CChar>, | |||
| _ oflag: Int32, | |||
There was a problem hiding this comment.
You've shimmed open() with the mode on Windows, why did you have to hide this overload?
There was a problem hiding this comment.
Because theres no openat equivalent atm. This is possible to do and unhide this, but, Im trying to get an immediate port going. It would be possible to get an implementation of openat and then unhide this in a follow up.
There was a problem hiding this comment.
But this is not openat, this is open with a file mode...
There was a problem hiding this comment.
Sorry, I meant, that because it calls openat. Now, arguably that is a bug, and we should fix that :-).
There was a problem hiding this comment.
Sorry, but I think it calls _swift_Platform_open: https://github.com/compnerd/apple-swift/blob/34777d0718cce6604d52353c25b69b48f32392e7/stdlib/public/Platform/Platform.swift#L218
There was a problem hiding this comment.
Ugh, I see what you mean. This was supposed to have an alternate implementation with the signature change. Ill fix this in the upload that Im about to do.
|
@swift-ci Please test OS X platform |
This adds the swiftMSVCRT module which is similar in spirit to swiftGlibc and swiftDarwin, exposing the Microsoft C Runtime library to swift. Furthermore, disable pieces of the standard library which are not immediately trivially portable to Windows. A lot of this functionality can still be implemented and exposed to the user, however, this is the quickest means to a PoC for native windows support. As a temporary solution, add a -DCYGWIN flag to indicate that we are building for the cygwin windows target. This allows us to continue supporting the cygwin environment whilst making the windows port work natively against the windows environment (msvc). Eventually, that will hopefully be replaced with an environment check in swift.
|
@swift-ci Please test |
|
@swift-ci Please test OS X platform |
|
@swift-ci Please test and merge |
1 similar comment
|
@swift-ci Please test and merge |
What's in this pull request?
Resolved bug number: (SR-)
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.
This adds the swiftMSVCRT module which is similar in spirit to swiftGlibc and
swiftDarwin, exposing the Microsoft C Runtime library to swift. Furthermore,
disable pieces of the standard library which are not immediately trivially
portable to Windows. A lot of this functionality can still be implemented and
exposed to the user, however, this is the quickest means to a PoC for native
windows support.
As a temporary solution, add a -DCYGWIN flag to indicate that we are building
for the cygwin windows target. This allows us to continue supporting the cygwin
environment whilst making the windows port work natively against the windows
environment (msvc). Eventually, that will hopefully be replaced with an
environment check in swift.