Skip to content
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

Rewrite SimpleDidSetRequest to not require a typechecked body #69215

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

DougGregor
Copy link
Member

SimpleDidSetRequest's implementation was depending on type-checking the body of the observer, meaning that computing the interface type of a didSet would require type-checking the body first. This triggers some request cycles, and is unnecessarily complicated.

Rewrite the implementation to work on a non-type-checked body, by using name lookup to establish when the oldValue parameter is referenced from the body.

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

Copy link
Contributor

@slavapestov slavapestov left a comment

Choose a reason for hiding this comment

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

Perhaps one day we can model oldValue as a capture or something, so that interface type computation does not depend on the contents of didSet's body.

@DougGregor
Copy link
Member Author

@swift-ci please smoke test macOS

@DougGregor
Copy link
Member Author

@swift-ci please clean smoke test macOS

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor DougGregor force-pushed the simpler-simple-did-set-request branch from e37f2a0 to 6f338ad Compare October 17, 2023 17:09
@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@xedin xedin removed their request for review October 17, 2023 21:07
SimpleDidSetRequest's implementation was depending on type-checking
the body of the observer, meaning that computing the interface type of
a `didSet` would require type-checking the body first. This triggers
some request cycles, and is unnecessarily complicated.

Rewrite the implementation to work on a non-type-checked body, by
using name lookup to establish when the `oldValue` parameter is
referenced from the body.
`buildTypeRefinementContextHierarchyDelayed()` is already guarded
behind a request, so it doesn't need its own logic for eliminating
redundant computation. Remove that logic, which incorrectly assumed
that the TRC was built for a function if the function body has already
been parsed.
@DougGregor DougGregor force-pushed the simpler-simple-did-set-request branch from 6f338ad to df6d501 Compare October 19, 2023 01:27
@DougGregor DougGregor requested a review from tshortli as a code owner October 19, 2023 01:27
@DougGregor
Copy link
Member Author

@swift-ci please smoke test

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