-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
[CSSolver] Hide witness overloads behind protocol requirements they match #13986
Conversation
…atch If overload set contains both protocol requirement and its witnesses let's only leave requirement itself active until we know that it matches, in such case we'd re-enable witness methods and attempt them. This helps to significantly reduce the number of overload choices we consider upfront e.g. for '==' operator we can hide 53 choices behind Equatable protocol from total number of 68 choices.
/cc @DougGregor please take a look, this implements raw folding of the witnesses behind protocol requirement overloads. |
@swift-ci please test compiler performance |
@swift-ci please test source compatibility |
@swift-ci Please smoke test compiler performance |
@xedin I've never had much luck with the full perf test, but smoke test seems to work most of the time. |
@xedin But in this case at least it looks like the failures are real :) Eg,
|
@slavapestov Thanks! This needs some more work, just wanted to get preliminary numbers since stdlib compiled successfully :) |
Build comment file:Summary for master smoketestUnexpected test results, stats may be off for 3 Regressions found (see below) Debugdebug briefRegressed (0)
Improved (2)
Unchanged (delta < 1.0% or delta < 100.0ms) (0)
debug detailedRegressed (2)
Improved (21)
Unchanged (delta < 1.0% or delta < 100.0ms) (0)
Releaserelease briefRegressed (0)
Improved (2)
Unchanged (delta < 1.0% or delta < 100.0ms) (0)
release detailedRegressed (0)
Improved (22)
Unchanged (delta < 1.0% or delta < 100.0ms) (1)
|
Again, this is technically a behavior change. I would appreciate a test case that shows we're acknowledging that. |
@jrose-apple Absolutely! I'm just trying to figure out if this is going to work at all, but at the end of the day, I'll absolutely try to come up with such test-case and add it to the suite. |
If overload set contains both protocol requirement and its witnesses
let's only leave requirement itself active until we know that it matches,
in such case we'd re-enable witness methods and attempt them. This
helps to significantly reduce the number of overload choices we consider
upfront e.g. for '==' operator we can hide 53 choices behind Equatable
protocol from total number of 68 choices.