-
Notifications
You must be signed in to change notification settings - Fork 10.6k
Remove "illegal" UnsafePointer casts from the stdlib. #3824
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
Conversation
Update for SE-0107: UnsafeRawPointer This adds a "mutating" initialize to UnsafePointer to make Immutable -> Mutable conversions explicit. These are quick fixes to stdlib, overlays, and test cases that are necessary in order to remove arbitrary UnsafePointer conversions. Many cases can be expressed better up by reworking the surrounding code, but we first need a working starting point.
|
@gribozavr What is holding up merging this? It will conflict with my changes and I have to start getting my own pull requests merged and CI tested. Playgrounds support is not updated for AnyHashable: |
|
@swift-ci Please test |
|
Let me review this PR. |
| let fileName = String(cString: utf8.baseAddress!) | ||
| return (fd, fileName) | ||
| return utf8.baseAddress!.withMemoryRebound( | ||
| to: CChar.self, capacity: Int(suffixlen)) { |
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.
Would you mind putting the ") {" on a separate line? Then you won't need a blank line.
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.
Actually, you can use String.nulTerminatedUTF8CString, and then you won't need to rebind.
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.
#3816 was merged, and the property nulTerminatedUTF8 is gone. Please switch to utf8CString and I think you won't need to rebind the memory then.
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.
Already reworked in #3816. I'll go through and annotate other such instances here. (Sorry if I'm stepping on toes; ignore if this isn't helpful.)
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.
Yeah, that's why I wanted your PR merged first. Unfortunately I didn't have permission to do it so had to post this older patch.
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 this rebind is gone now.
Incidentally, I much prefer your suggested style, but I can never keep track of what we're supposed to do. Is there a style guideline for stdlib code?
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.
Sorry, we don't have a written document.
This would be the most complex case of wrapping the declaration:
func foo<
T,
U : Hashable // Either put all type parameters on one line, on do one per line.
>( // Closing delimiters are put on a new line.
arg1 x: T,
arg2 y: U // Same, either everything fits on one line, or each gets its own line.
) -> Int
where T.Z == U
{
...
}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.
Nice. Those are good simple rules. I think that would suffice in a document or a Readme.md
|
This fails on Linux: |
|
Please see #3843 |
Update for SE-0107: UnsafeRawPointer
This adds a "mutating" initialize to UnsafePointer to make
Immutable -> Mutable conversions explicit.
These are quick fixes to stdlib, overlays, and test cases that are necessary
in order to remove arbitrary UnsafePointer conversions.
Many cases can be expressed better up by reworking the surrounding
code, but we first need a working starting point.