-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[5.0][stdlib][SR-2239]: add support for AArch64 variadics #20863
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
[5.0][stdlib][SR-2239]: add support for AArch64 variadics #20863
Conversation
@swift-ci test |
This can land after #20877 is merged |
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.
The corresponding PR for master got a number of review comments. I couldn't tell if those concerns have been addressed (because the PR was merged before the review), but I think not. We should make sure we're happy with the changes and get any follow-up commits included here before merging this.
@bob-wilson @tkremenek I have assessed the comments of the reviewers of the PR for the master and I have found no issues that need to be addressed before this PR can be merged. This code and the other related PR's have been thoroughly tested on the target platform before submitting. Suggestions about refactoring code that effect platforms other than AArch64/Linux can be best addressed in future PR's. |
All PRs with the pending-convergence label are being held for merging. The |
@futurejones Perhaps other open-source projects work differently, but at least for Swift, the expectation is that you will come to an agreement during the code review process. When I look at #20862 I see a bunch of requests for changes that you marked as "resolved" without either changing anything or convincing others to accept your opinion that those changes should be done later. That is not how it works. We really do want your contributions to Swift and I know it can be frustrating to be asked for follow-up changes when you're just trying to get it to work, but we also have to watch out for the long-term quality of the code. Please work with the reviewers to reach a consensus about what to do on the master branch. If that doesn't happen soon, I think we should revert the change from master until the review is completed. |
@bob-wilson please see my comments on #20862 @tkremenek can we move forward on this please. |
I will defer to the reviewers of #21237 whether they think this should go in to the 5.0 branch with or without those changes (or perhaps a subset of them). |
[Stdlib] Rename AnyKeyPath's ObjC name and _VaListBuilder to avoid conflicts with older stdlibs. #21127
Is this PR obsolete now that we've merged #21885? |
In my opinion, this one is obsolete after merging #21885 (thanks for that, btw). |
@bob-wilson this PR is no longer needed as the changes have been included in #21885. |
Cherry-pick of: #20862
Adds support for AArch64 variadics.
Resolves SR-2239.
Previous issues preventing building of the
swift-5.0-branch
have been fixed by defaulting to the "gold" linker.This has been updated in #20846
We are now seeing successful builds of the
swift-5.0-branch
on AArch64 Linux Ubuntu 16.04http://futurejones.xyz:8080/job/swift-5.0-aarch64/