Optimizer: fix handling of dependent existential archetypes in alloc_stack and apply simplification
#83898
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
|
@swift-ci test |
eeckstein
requested review from
aidan-hall,
atrick,
meg-gupta and
nate-chandler
and removed request for
hborla and
xedin
slavapestov
requested changes
Aug 25, 2025
| guard substitutionMap.replacementTypes.contains(where: { $0.isExistentialArchetype }), | ||
| substitutionMap.replacementTypes.allSatisfy({ $0.isExistentialArchetype || !$0.hasLocalArchetype }) | ||
| // TODO: support non-root existential archetypes | ||
| guard substitutionMap.replacementTypes.contains(where: { $0.isRootExistentialArchetype }), |
There was a problem hiding this comment.
Why is this a precondition?
| return type.isRootExistentialArchetype || | ||
| // case 2. the argument _is_ a metatype of the existential archetype | ||
| (type.isMetatype && type.canonicalType.instanceTypeOfMetatype.isExistentialArchetype) || | ||
| (type.isMetatype && type.canonicalType.instanceTypeOfMetatype.isRootExistentialArchetype) || |
There was a problem hiding this comment.
Can't you handle all the cases by transforming the type recursively?
| /// ``` | ||
| func optimizeExistential(_ context: SimplifyContext) -> Bool { | ||
| guard type.isExistential || type.isExistentialArchetype, | ||
| // TODO: support non-root existential archetypes |
There was a problem hiding this comment.
I would prefer we just fixed this with the right abstractions. You can get this from the conformance of Self to the protocol, stored in the init_existential instruction.
|
@slavapestov we can think of making this improvements in a follow-up PR. But this PR is intended to be cherry-picked to 6.2 and should be at lowest risk. That means: just adding additional bail-out conditions |
slavapestov
approved these changes
Aug 25, 2025
|
@swift-ci smoke test windows |
…_stack` and `apply` simplification When replacing an opened existential type with the concrete type, we didn't consider that the existential archetype can also be a "dependent" type of the root archetype. For now, just bail in this case. In future we can support dependent archetypes as well. Fixes a compiler crash. rdar://158594365
eeckstein
force-pushed
the
fix-simplify-alloc-stack-apply
branch
from
c1cce39 to
56521f0
Compare
|
@swift-ci smoke test |
This was referenced Aug 27, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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 replacing an opened existential type with the concrete type, we didn't consider that the existential archetype can also be a "dependent" type of the root archetype.
For now, just bail in this case. In future we can support dependent archetypes as well.
Fixes a compiler crash.
rdar://158594365