Skip to content

[4.0] Escalate the warning about the "== Self" type soundness hole to an error #10320

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

DougGregor
Copy link
Member

@DougGregor DougGregor commented Jun 16, 2017

Explanation: Swift 3 had a type soundness hole in protocol conformance checking where the requirement contained an `== Self" constraint and the witness was a member of a non-final class. We closed this hole in Swift 4, but left it as a warning in Swift 3.2. Unfortunately, this soundness hole breaks invariants that the SIL optimizer depends on, so the optimizer will crash. Escalate the warning to an error even in Swift 3.2 mode to close this type hole, because it's better to reject a program than crash trying to compile it.
Scope: We know of one project that will start to be rejected in Swift 3.2 mode because of this change, because that project is (unintentionally) exploiting the type soundness hole. Today, that project can only compile as Swift 3.2 and only with the optimizers turned off.
Radar: rdar://problem/28601761
Risk: Very low. The risk to anyone not depending on this type soundness hole is zero; some projects that were depending on this type soundness hole will now fail to compile (with a Fix-It to suggest how to avoid the hole).
Testing: Compiler regression testing.

@DougGregor DougGregor added this to the Swift 4.0 milestone Jun 16, 2017
@DougGregor
Copy link
Member Author

@swift-ci please test

// Note: while Swift 3.2 originally intended to provide backward
compatibility here, the type-soundness issue was considered so severe
(due to it breaking the optimizer) that that we escalated it to an
error.
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing //

Copy link
Member Author

Choose a reason for hiding this comment

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

muahahaha

Copy link
Member Author

Choose a reason for hiding this comment

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

thank you

@DougGregor DougGregor force-pushed the escalate-type-soundness-warning-4.0 branch from 9919a09 to 80f9dd8 Compare June 16, 2017 17:49
@DougGregor DougGregor closed this Jun 16, 2017
@DougGregor DougGregor reopened this Jun 16, 2017
@DougGregor
Copy link
Member Author

@swift-ci please test

2 similar comments
@DougGregor
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member Author

@swift-ci please test

Copy link
Contributor

@jckarter jckarter left a comment

Choose a reason for hiding this comment

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

Sounds like the right tradeoff to me.

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 80f9dd82b16358748857ebfa3b09ed4cf2d611df
Test requested by - @DougGregor

…ror.

Swift 3 had a type soundness hole in protocol conformance checking
where the requirement contained an "== Self" constraint and the
witness was a member of a non-final class. We previously closed the
type soundness hole in PR swiftlang#9830, but left it as a warning in Swift 3
compatibility mode.

Escalate that warning to an error. The optimizers break due to this
type soundness hole, and of course it can lead to other runtime
breakage because it violates the type system.

Fixes rdar://problem/28601761.

(cherry picked from commit 8b80681)
@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 80f9dd82b16358748857ebfa3b09ed4cf2d611df
Test requested by - @DougGregor

@DougGregor DougGregor force-pushed the escalate-type-soundness-warning-4.0 branch from 80f9dd8 to 5638d92 Compare June 16, 2017 22:01
@DougGregor
Copy link
Member Author

@swift-ci please test and merge

1 similar comment
@DougGregor
Copy link
Member Author

@swift-ci please test and merge

@swift-ci swift-ci merged commit 128f341 into swiftlang:swift-4.0-branch Jun 16, 2017
@DougGregor DougGregor deleted the escalate-type-soundness-warning-4.0 branch June 16, 2017 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants