Skip to content

[5.0] Get the ModelAssistant source compatibility project building with the swift-5.0-branch #20971

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Dec 5, 2018
Merged

[5.0] Get the ModelAssistant source compatibility project building with the swift-5.0-branch #20971

merged 2 commits into from
Dec 5, 2018

Conversation

rudkx
Copy link
Contributor

@rudkx rudkx commented Dec 3, 2018 •
edited
Loading

Explanation: There were a couple issues with how rethrows diagnostics were dealing with optional function types, resulting in a verification issue in a new source compatibility suite project.

Issue: SR-9102 / rdar://problem/45615204

Scope: Only affects code that has arguments with are optional throwing function types.

Risk: Low.

Testing: Added a new test for the case where we were failing verification.

Reviewed by: @rjmccall, @xedin

Does not affect ABI.

rudkx added 2 commits December 3, 2018 11:13
@rudkx
[ConstraintSystem] Do not infer 'nil' arguments as optional throwing …
e46c5cb 
…function types.

I have a follow-up change to correctly classify optional arguments,
and with that fix applied we incorrectly emit rethrow diagnostics for
'nil' arguments passed in places where optional throwing functions are
expected.

This change ensures that we strip the throwing bit off of the nested
function type so that we don't consider 'nil' arguments in these
positions as potentially throwing functions.

(cherry picked from commit 1ed46b4)
@rudkx
[TypeChecker] Fix a problem with rethrows classification.
9099ad9 
We failed to properly classify arguments that are of optional function
type, in this case leading to a verification failure due to the throws
attribute on the apply expression not being set to some value.

Fixes: SR-9102 / rdar://problem/45615204
(cherry picked from commit 5a498db)
@rudkx rudkx requested a review from a team as a code owner December 3, 2018 19:27
@rudkx rudkx changed the title Get the ModelAssistant source compatibility project building with the swift-5.0-branch [5.0] Get the ModelAssistant source compatibility project building with the swift-5.0-branch Dec 3, 2018
@rudkx
Copy link
Contributor Author

rudkx commented Dec 3, 2018

@swift-ci Please test

@rudkx
Copy link
Contributor Author

rudkx commented Dec 3, 2018

@swift-ci Please test source compatibility

AnnaZaks
AnnaZaks approved these changes Dec 3, 2018
View reviewed changes
Copy link
Contributor

@AnnaZaks AnnaZaks left a comment

Choose a reason for hiding this comment

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

Approved for after convergence.

@rudkx rudkx mentioned this pull request Dec 4, 2018
Merged
@rudkx
Copy link
Contributor Author

rudkx commented Dec 4, 2018

@shahmishal IIRC, cross-repo testing doesn't work with the source compatibility suite. Am I remembering that correctly?

@shahmishal
Copy link
Member

swiftlang/swift-source-compat-suite#298

@swift-ci Please test source compatibility

@rudkx
Copy link
Contributor Author

rudkx commented Dec 4, 2018

The source compatibility failures consist of a UPASS that will go away once we merge swiftlang/swift-source-compat-suite#298, and then a few UPASSes that are unrelated to my change and reproduce without it.

@rudkx
Copy link
Contributor Author

rudkx commented Dec 4, 2018

@swift-ci Please test source compatibility

@rudkx
Copy link
Contributor Author

rudkx commented Dec 4, 2018

@swift-ci Please test

@rudkx
Copy link
Contributor Author

rudkx commented Dec 5, 2018

@AnnaZaks This should be good to merge. Most of the UPASS results in the source compatibility suite are unrelated to this change, e.g. they also show up here: https://ci.swift.org/job/swift-PR-source-compat-suite-debug/444/

The one that is related to this change will go away once we merge this and then swiftlang/swift-source-compat-suite#298.

@AnnaZaks AnnaZaks merged commit a9dba6a into swiftlang:swift-5.0-branch Dec 5, 2018
@rudkx rudkx deleted the fix-sr9102-5.0 branch December 5, 2018 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AnnaZaks AnnaZaks AnnaZaks approved these changes

Assignees
No one assigned
Labels
pending convergence
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rudkx @shahmishal @AnnaZaks