Register Conformances as Potential Member Constraints #30585
Merged
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
Unwind a hack whose stated purpose was to register a potential member edge from an extension to the extended type. In reality, this only registered a plain member dependency on 'deinit'. This edge is insufficient in isolation to cause a rebuild of a dependent file in the case where a type and its extension live in separate files. However, we appear to have been saved by the redundancy in edge registration because the lookup for the extended type will register a top-level or nominal dependency (for an unqualified or qualified reference respectively). The worry there is if a protocol conformance edge *should* flip a previously private nominal dependency edge to a cascading edge. In such a case, the old code would not have been able to make the cascading edge promotion, and we would have potentially miscompiled by not rescheduling dependent jobs.
|
@swift-ci test |
|
This whole debacle strongly suggests we should consider adding explicit dependency types for protocol conformances and extensions. |
slavapestov
approved these changes
Mar 23, 2020
davidungar
approved these changes
Mar 23, 2020
| // don't care about /any/ of the type's members, only that it conforms to | ||
| // the protocol. | ||
| tracker->addUsedMember({Adoptee, DeclBaseName::createDestructor()}, | ||
| tracker->addUsedMember({Adoptee, Identifier()}, |
There was a problem hiding this comment.
I'd like to see a comment here explaining why the potentialMember dep is needed.
There was a problem hiding this comment.
I'll tag you as a reviewer for a follow-up.
|
⛵️ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Unwind a hack whose stated purpose was to register a potential member
edge from an extension to the extended type. In reality, this only
registered a plain member dependency on 'deinit'. This edge is
insufficient in isolation to cause a rebuild of a dependent file in the
case where a type and its extension live in separate files. However, we
appear to have been saved by the redundancy in edge registration because the
lookup for the extended type will register a top-level or nominal
dependency (for an unqualified or qualified reference respectively). The
worry there is if a protocol conformance should flip a previously
private nominal dependency edge to a cascading edge. In such a case, the
old code would not have been able to make the cascading edge promotion,
and we would have potentially miscompiled by not rescheduling dependent
jobs.