[Type checker] Minimize checking needed to compute the set of overridden declarations #17729
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Rather than deferring to the heavyweight validateDeclForNameLookup() to perform the “get overridden decls” operation, perform a much more minimal validation that only looks for exact matches when the ‘override’ modifier is present. The more-extensive checking (e.g., that one didn’t forget an override modifier) is only performed as part of the “full” type checking of a declaration, which will typically only be done within the same source file as the declaration. The intent here is to reduce the amount of work performed to check overrides cross-file, and limit the dependencies of the “get overridden decls” operation.
Member
Author
|
Note that two tests are currently failing, so this PR isn't quite complete yet: |
Member
Author
|
This PR is getting too big; I've split out the refactoring part as #17733 |
'dynamic' is inherited and only applies to '@objc' entities. The SourceKit test change is because we've changed our behavior slightly: we won't infer @objc from an override unless the override is correctly marked with 'override'. This is part of breaking up dependencies, and should have little effect outside of tests.
DougGregor
force-pushed
the
minimize-override-checking
branch
from
e2bfbcc to
a3e03fa
Compare
Member
Author
|
@swift-ci please smoke test |
Member
Author
|
@swift-ci please test source compatibility |
Member
Author
|
@swift-ci please smoke test |
Member
Author
|
@swift-ci please test source compatibility |
Member
Author
|
@swift-ci please smoke test macOS |
1 similar comment
Member
Author
|
@swift-ci please smoke test macOS |
When computing the type of a potentially-overriden declaration, make sure we have an interface type. Add a test to ensure that we validate overrides cross-file correctly.
Member
Author
|
@swift-ci please smoke test |
Member
Author
|
@swift-ci please test source compatibility |
Member
Author
|
@swift-ci please smoke test |
Member
Author
|
@swift-ci please test source compatibility |
Member
Author
|
@swift-ci please test compiler performance |
1 similar comment
Member
Author
|
@swift-ci please test compiler performance |
Contributor
Build comment file:Compilation-performance test failed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor the checking of overridden declarations so we only perform the minimal work to answer the question "what declarations to this declaration override?". Includes a healthy refactoring of the previously-monolithic
TypeChecker::checkOverrides()into re-usable pieces that are far easier to reason about than before.