[CSSimplify] Parameter pack wrapping logic incorrectly considers tuple LValueTypes to not be tuples
#85962
Conversation
ultramiraculous
requested review from
hborla,
slavapestov and
xedin
as code owners
lib/Sema/CSSimplify.cpp
Outdated
| if (isa<TupleType>(desugar1) != isa<TupleType>(desugar2) && | ||
|
|
||
| // Ignore the l-valueness when checking for wrapping | ||
| bool type1IsTuple = desugar1->getWithoutSpecifierType()->is<TupleType>(); |
There was a problem hiding this comment.
Previously isa<TupleType> would always return false when it it the LValueType, leading to this check erroneously passing:
isa<TupleType>(desugar1) != isa<TupleType>(desugar2)
In my examples the left value would not be diagnoised as a tuple here or in the subsequent matchTypes nested call, leading to a TupleType getting allocated with just the @lValue type.
There was a problem hiding this comment.
I don't think unwrapping is the right move here, I think we just need to delay this just like we did for InOutType down below with !isa<LValueType>(desugar1) && !isa<LValueType>(desugar2) && because match types would handle LValue -> RValue and LValue -> LValue type already by recursively calling itself.
There was a problem hiding this comment.
Ok yeah I think I found the conversion code when I was trying to find getWithoutSpecifierType but didn't register what it was for.
Let me see if that works.
There was a problem hiding this comment.
Looks like the tests still pass with this change (and fail without it, thank god 😅)!
slavapestov
left a comment
slavapestov
left a comment
There was a problem hiding this comment.
This looks reasonable, I think. Maybe @xedin can give a second opinion
|
@swift-ci Please smoke test |
|
@slavapestov I updated the comments as mentioned, but also realized I forgot to change a test name. This might need another @swift-ci Please smoke test |
|
@swift-ci Please smoke test |
|
@swift-ci please test |
|
@swift-ci please test Windows platform |
…e `LValueType`s to not be tuples (swiftlang#85962) In swiftlang#65125 (and beyond) `matchTypes`, has logic to attempt to wrap an incoming parameter in a tuple under certain conditions that might help with type expansion. In the case the incoming type was backed by a `var`, it would be wrapped by an `LValueType` then be subsequently mis-diagnosed as not-a-tuple. More details in swiftlang#85924 , this this is also the cause of (and fix for) swiftlang#85837 as well...
[6.3][CSSimplify] Parameter pack wrapping logic incorrectly considers tuple LValueTypes to not be tuples #85962
3 participants
In #65125 (and beyond)
matchTypes, has logic to attempt to wrap an incoming parameter in a tuple under certain conditions that might help with type expansion.In the case the incoming type was backed by a
var, it would be wrapped by anLValueTypethen be subsequently mis-diagnosed as not-a-tuple.More details in #85924 , this this is also the cause of (and fix for) #85837 as well...