Skip to content

Diagnose unsound uses of same-type-to-Self requirements with non-final classes #9830

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

Merged
merged 1 commit into from
May 22, 2017

Conversation

DougGregor
Copy link
Member

Protocol requirements involving same-type-to-Self constraints cannot
be witnessed by declarations in non-final classes that have the same
form of same-type requirement to the corresponding class type, because
it creates a soundness hole with subclasses:

protocol Q {
  func foo<T: P>(_: T, _: T.T) where T.T == Self
}

class C: Q {
  func foo<T: P>(_: T, _: C) where T.T == C {}
}

class D: C {
  // in D, T.T == D does not hold
}

Warn about this in Swift 3 compatibility mode, error on it in Swift 4
mode. When possible, provide a note + Fix-It suggesting that the
same-type constraint might be weakened to a superclass constraint
(which works with subclassing).

Fixes rdar://problem/30398503.

…l classes.

Protocol requirements involving same-type-to-Self constraints cannot
be witnessed by declarations in non-final classes that have the same
form of same-type requirement to the corresponding class type, because
it creates a soundness hole with subclasses:

    protocol Q {
      func foo<T: P>(_: T, _: T.T) where T.T == Self
    }

    class C: Q {
      func foo<T: P>(_: T, _: C) where T.T == C {}
    }

    class D: C {
      // in D, T.T == D does not hold
    }

Warn about this in Swift 3 compatibility mode, error on it in Swift 4
mode. When possible, provide a note + Fix-It suggesting that the
same-type constraint might be weakened to a superclass constraint
(which works with subclassing).

Fixes rdar://problem/30398503.
@DougGregor
Copy link
Member Author

@swift-ci please smoke test and merge

@swift-ci swift-ci merged commit 033a38e into swiftlang:master May 22, 2017
@DougGregor DougGregor deleted the same-type-self-soundness-hole branch May 22, 2017 21:05
@swiftix
Copy link
Contributor

swiftix commented May 25, 2017

@DougGregor Nice! This should have resulted in compile-time warnings when compiling ProcedureKit, which is in the compatibility suite now. Do you know if this PR has caught any bugs there?

@lplarson Luke do you know if we got any errors/warning about it in ProcedureKit after this PR was merged?

@swiftix
Copy link
Contributor

swiftix commented May 25, 2017

Looks like it did!

/Users/buildslave/jenkins/workspace-private/swift-master-source-compat-suite-assert/project_cache/ProcedureKit/Sources/ProcedureKit/Procedure.swift:1108:15: warning: instance method 'add(observer:)' in non-final class 'Procedure' cannot be used to satisfy requirement instance method 'add(observer:)' (in protocol 'ProcedureProtocol') due to same-type requirement involving 'Self'
    open func add<Observer: ProcedureObserver>(observer: Observer) where Observer.Procedure == Procedure {
              ^
/Users/buildslave/jenkins/workspace-private/swift-master-source-compat-suite-assert/project_cache/ProcedureKit/Sources/ProcedureKit/Procedure.swift:1108:93: note: consider weakening the same-type requirement 'Observer.Procedure' == 'Procedure' to a superclass requirement
    open func add<Observer: ProcedureObserver>(observer: Observer) where Observer.Procedure == Procedure {

@swiftix
Copy link
Contributor

swiftix commented May 25, 2017

@DougGregor I've commented on the radar that eventually we want to make it an error even in Swift 3 compatibility mode.

DougGregor added a commit to DougGregor/swift that referenced this pull request Jun 16, 2017
…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.
DougGregor added a commit to DougGregor/swift that referenced this pull request Jun 16, 2017
…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)
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.

3 participants