Skip to content

[CodeCompletion] Avoid creating circles in the USRBasedType supertype hierarchy #58522

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 3, 2022

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Apr 29, 2022

You would think that superclass + conformances form a DAG. You are wrong!
We can achieve a circular supertype hierarcy with

protocol Proto : Class {}
class Class : Proto {}

USRBasedType is not set up for this. Serialization of code completion results from global modules can't handle cycles in the supertype hierarchy
because it writes the DAG leaf to root(s) and needs to know the type offsets. To get consistent results independent of where we start
constructing USRBasedTypes, ignore superclasses of protocols. If we kept track of already visited types, we would get different results depending on
whether we start constructing the USRBasedType hierarchy from Proto or Class.
Ignoring superclasses of protocols is safe to do because USRBasedType is an under-approximation anyway.

rdar://91765262

… hierarchy

You would think that superclass + conformances form a DAG. You are wrong!
We can achieve a circular supertype hierarcy with

```swift
protocol Proto : Class {}
class Class : Proto {}
```

USRBasedType is not set up for this. Serialization of code completion results from global modules can't handle cycles in the supertype hierarchy
because it writes the DAG leaf to root(s) and needs to know the type offsets. To get consistent results independent of where we start
constructing USRBasedTypes, ignore superclasses of protocols. If we kept track of already visited types, we would get different results depending on
whether we start constructing the USRBasedType hierarchy from Proto or Class.
Ignoring superclasses of protocols is safe to do because USRBasedType is an under-approximation anyway.

rdar://91765262
@ahoppen ahoppen requested a review from bnbarham April 29, 2022 10:53
@ahoppen
Copy link
Member Author

ahoppen commented Apr 29, 2022

@swift-ci Please smoke test

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

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

It's weird that we specify Class as a superclass of Proto here. As far as I can tell, it's actually just a requirement that conforming types must either be Class or inherit from Class. So the fix is fine, but it'd be nice if it was modeled differently in the AST.

@ahoppen ahoppen merged commit eb8df86 into swiftlang:main May 3, 2022
@ahoppen ahoppen deleted the pr/no-circular-supertypes branch May 3, 2022 07:43
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