Serialization: Don't serialize conformances that should be skipped #68262
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
Contributor
Author
|
@swift-ci please test |
xymus
approved these changes
Sep 1, 2023
When `-experimental-serialize-external-decls-only` is specified, skip serializing conformances to protocols that should be skipped to avoid unnecessary typechecking. Also, ensure type and value witnesses are resolved lazily during serialization by passing `true` for `useResolver`. Resolves rdar://114799742
tshortli
force-pushed
the
emit-module-lazy-typecheck-conformances
branch
from
11f1be5 to
fde2016
Compare
Contributor
Author
|
@swift-ci please smoke test |
tshortli
added a commit
to tshortli/swift
that referenced
this pull request
Jan 1, 2024
Serialization now enumerates the value witnesses of conformances with a resolver in order to facilitate lazy type checking (swiftlang#68262). This change in behavior introduced an assertion failure when compiling modules from swiftinterface when the interface contains a conformance to an `@objc` protocol that has a requirement that is imported with an `async` variant. The `forEachValueWitness()` invocation during serialization was causing the missing witness for an objc async protocol requirement to be resolved after conformance check was already marked "complete". This witness had not been previously resolved because `ConformanceChecker::resolveValueWitnesses()` had special logic to skip objc protocol requirements with witnessed siblings, whereas `forEachValueWitness()` did not. There are multiple potential solutions to this problem, but the one that seemed least disruptive to me was to stop skipping resolution of these sibling witnesses during `ConformanceChecker::resolveValueWitnesses()`. When I looked into why they were being skipped, I discovered that this seemed to be a concession to bugs in the logic for pruning missing witnesses. When I adjusted the pruning logic it no longer became necessary to skip witnesses in `ConformanceChecker::resolveValueWitnesses()` in order to avoid incorrect diagnostics. Resolves rdar://119435253
Catfish-Man
pushed a commit
to Catfish-Man/swift
that referenced
this pull request
Jan 19, 2024
Serialization now enumerates the value witnesses of conformances with a resolver in order to facilitate lazy type checking (swiftlang#68262). This change in behavior introduced an assertion failure when compiling modules from swiftinterface when the interface contains a conformance to an `@objc` protocol that has a requirement that is imported with an `async` variant. The `forEachValueWitness()` invocation during serialization was causing the missing witness for an objc async protocol requirement to be resolved after conformance check was already marked "complete". This witness had not been previously resolved because `ConformanceChecker::resolveValueWitnesses()` had special logic to skip objc protocol requirements with witnessed siblings, whereas `forEachValueWitness()` did not. There are multiple potential solutions to this problem, but the one that seemed least disruptive to me was to stop skipping resolution of these sibling witnesses during `ConformanceChecker::resolveValueWitnesses()`. When I looked into why they were being skipped, I discovered that this seemed to be a concession to bugs in the logic for pruning missing witnesses. When I adjusted the pruning logic it no longer became necessary to skip witnesses in `ConformanceChecker::resolveValueWitnesses()` in order to avoid incorrect diagnostics. Resolves rdar://119435253
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.
When
-experimental-serialize-external-decls-onlyis specified, skip serializing conformances to protocols that should be skipped to avoid unnecessary typechecking. Also, ensure type and value witnesses are resolved lazily during serialization by passingtrueforuseResolver.Resolves rdar://114799742