Skip to content

AST: Remove ProtocolDecl::getSuperclass() #72495

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 11 commits into from
Mar 22, 2024

Conversation

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented Mar 21, 2024

Cleaning up my own technical debt from 2018.

There are two ways to spell a class-constrained protocol:

class C {}
protocol P1: C {}
protocol P2 where Self: C {}

The ProtocolDecl::getSuperclass() method only handled the first case, returning a null Type in the second case.

This remained an issue for as long as it did because most clients would either use the lower-level getSuperclassDecl(), which correctly visited the where clause, or they would process requirement signature requirements or whatever.

This problem surfaced again because there was a recent regression with the where Self: C form because a new call to getSuperclass() was introduced in serialization.

Also it turns out all existing other callers of getSuperclass() either suffered from the same problem, or were unnecessary. So this PR just removes ProtocolDecl::getSuperclass().

Fixes rdar://problem/124478687.

… ProtocolDecl::getSuperclass()

Protocols with a superclass bound written as `protocol P where Self: C`
return null from getSuperclass(). Unqualified lookup only cares about
getSuperclassDecl(), so serialize that instead.

Fixes rdar://problem/124478687.
…ete contraction

This changes the minimized signature in a very narrow edge case.

If you have

    class C : P {}

and also

    protocol P : C {}
    protocol P where Self : C {}

then <T where T : P, T : C> now becomes <T : P> both with spellings;
before, the first one gave <T : P> and the second <T : C>.
The condition that the conforming type satisfies the superclass bound
of the conformed protocol is already covered by the conformance constraint
itself.
@slavapestov slavapestov force-pushed the fix-protocol-superclass branch from 96414be to 1a7bdb4 Compare March 21, 2024 19:18
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov slavapestov merged commit 336750e into swiftlang:main Mar 22, 2024
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.

2 participants