-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[CS] Add special case to preserve compatibility for rdar://84279742 #39852
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When ranking constructor parameter lists, we compose them as tuples or parens, and check if they are subtypes or unlabeled versions of each other. Previously this was done with the parameter flags intact, but recently I changed the logic to explicitly strip parameter flags in preparation for no longer storing the flags on these types. This caused a slight behavior change, as it turns out we have a special case in `TupleType::get` that allows an unlabeled single parameter to be composed as a tuple type if its variadic bit is set. With the parameter flags now stripped, we produce a paren type. This means that when comparing the parameter lists e.g `(x: Int...)` and `(Int...)`, instead of comparing two tuple types end up comparing a tuple with a paren and fail. To preserve the old behavior, implement a special case for when we have an unlabeled and labeled variadic comparison for a single parameter. In this case, add the parameter types directly to the type diff, and track which one had the label. The ranking logic can then use this to prefer the unlabeled variant. This is only needed in the single parameter case, as other cases will compare as tuples the same as before. In cases where variadics aren't used, we may end up trying to compare parens with tuples, but that's consistent with what we previously did. rdar://84279742
@swift-ci please test |
@swift-ci please test source compatibility |
Just out of curious, I'd like to know why the change only effect Linux not macOS? It seems the lib/Sema/CSRanking.cpp is used both on Linux and macOS. @hamishknight |
@Kyle-Ye Hmm rdar://84279742 says it also failed for macOS. I'm not aware of any compiler behavior that would cause the ranking to differ between macOS and Linux, are there any |
Because my PR and other's PR are both failed on the Linux Platform not on the macOS Platform. I see the Linux Test is using the ubuntu1602-2021-10-17 snapshot of Swift but did not see which version the macOS Platform is using on the env. |
@Kyle-Ye Ah it looks like the Linux run is building off the latest main snapshot, however the macOS run appears to be using the CI toolchain, which will be an older version of Swift. That would explain the difference in behavior. FWIW I can confirm that using my local build of Swift on macOS, swift-markdown fails to build without these changes, and successfully builds with them. |
@swift-ci please smoke test Linux |
When ranking constructor parameter lists, we compose them as tuples or parens, and check if they are subtypes or unlabeled versions of each other. Previously this was done with the parameter flags intact, but recently in #39543 I changed the logic to explicitly strip parameter flags in preparation for no longer storing the flags on these types.
This caused a slight behavior change, as it turns out we have a special case in
TupleType::get
that allows an unlabeled single parameter to be composed as a tuple type if its variadic bit is set. With the parameter flags now stripped, we produce a paren type. This means that when comparing the parameter lists e.g(x: Int...)
and(Int...)
, instead of comparing two tuple types, we end up comparing a tuple with a paren and fail.To preserve the old behavior, implement a special case for when we have an unlabeled and labeled variadic comparison for a single parameter. In this case, add the parameter types directly to the type diff, and track which one had the label. The ranking logic can then use this to prefer the unlabeled variant. This is only needed in the single parameter case, as other cases will compare as tuples the same as before. In cases where variadics aren't used, we may end up trying to compare parens with tuples, but that's consistent with what we previously did.
SR-15362
rdar://84279742