[CS] Use simplifyType in isDependentMemberTypeWithBaseThatContainsUnresolvedPackExpansions
#84875
Conversation
…UnresolvedPackExpansions` The pack expansion type variable may be a nested in the fixed type of another type variable, and as such we unfortunately need to fully `simplifyType` here. rdar://162545380
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
| // this is called from `matchTypes`. We need to completely simplify the type | ||
| // though since pack expansions can be present in fixed types for nested | ||
| // type vars. | ||
| auto baseTy = cs.simplifyType(type->getDependentMemberRoot()); |
There was a problem hiding this comment.
If this is a hotspot, an alternate implementation is to call getFixedTypeRecursive() and recurse on each type variable that appears therein
There was a problem hiding this comment.
This is really unfortunate, I was trying to avoid it…
There was a problem hiding this comment.
If this is a hotspot, an alternate implementation is to call getFixedTypeRecursive() and recurse on each type variable that appears therein
Happy to try that instead
There was a problem hiding this comment.
Actually I wonder if we could fix this by moving this logic into the TypeSimplifier itself, I didn't actually realize that getFixedTypeRecursive is calling into simplifyType for DependentMemberType, seems like we ought to be able to change type simplification to decline to simplify a DMT base with an unresolved pack expansion
There was a problem hiding this comment.
The problem is that we delay matching only if the base is a type variable at the moment, that’s why this and the other check for tuples are directly in matchTypes, in this case the base is not a type variable. I think fix that you’d that to change the meaning of isTypeVariableOrMember()
There was a problem hiding this comment.
Hmm not sure I follow, for context this is the follow-up I'm planning on doing hamishknight@536304f, which seems to work fine
There was a problem hiding this comment.
I tried almost exactly that to fix the issue in #84729. I'm talking about check in - https://github.com/swiftlang/swift/blob/main/lib/Sema/CSSimplify.cpp#L7371-L7374, in the example I added the "base" isn't actually a type variable so even though the dependent member didn't get simplified we end up still attempting to match it against the other type and failing that's why I didn't end up going with that.
There was a problem hiding this comment.
Oh, I think the problem with my change might have been that it still produced a dependent member type with "newBase" instead of returning the original type... I'm a bit worried if we'd be in a situation that does have a partially resolved dependent member base type at some point though and that would cause too aggressive simplification in matchTypes...
|
Source compat failures are unrelated, will do a follow-up PR to clean up this logic |
|
@swift-ci please smoke test macOS |
3 participants
The pack expansion type variable may be a nested in the fixed type of another type variable, and as such we unfortunately need to fully
simplifyTypehere. This fixes a regression from #84729.rdar://162545380