-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Data: adjust for Windows #1903
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
Data: adjust for Windows #1903
Conversation
@swift-ci please test |
Foundation/Data.swift
Outdated
@@ -12,7 +12,7 @@ | |||
|
|||
#if DEPLOYMENT_RUNTIME_SWIFT | |||
|
|||
#if os(Linux) | |||
#if os(Linux) || os(Windows) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think my comment from #1868 (comment) is still valid and this should be #if !canImport(Darwin)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mmm, pretty sure that FreeBSD also provides malloc_good_size
... so, I can spell it out as the inverse if you like, but I think that this is better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that we should be more general where possible. !canImport(Darwin) && !os(FreeBSD)
maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the man pages at https://man.openbsd.org/FreeBSD-12.0/malloc_good_size (which seem to support multiple OSes) no other OS incl FreeBSD has malloc_good_size
even though it says FreeBSD's malloc
page shows it uses jemalloc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there support for os(FreeBSD)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, there is support for os(FreeBSD)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We talked a bit off PR.
I think that this block relies on (not having) the interface of the API offered by the Darwin
module. So since the Darwin module is imported earlier if canImport(Darwin)
is true, and we are assuming that the Darwin API is implicitly coming from that module, then !canImport(Darwin)
is correct here.
We also looked into FreeBSD docs, and it looks like it doesn't expose exactly that symbol. (jemalloc
, its malloc implementation, may have a similar one, but it isn't documented API). Let's just use !canImport(Darwin)
here, and let FreeBSD port champions see if a different implementation can be used.
`malloc_good_size` is not available on Windows. Additionally, `munmap` is spelt `UnmapViewOfFile`.
@swift-ci please test and merge |
malloc_good_size
is not available on Windows. Additionally,munmap
is spelt
UnmapViewOfFile
.