-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #3252: Change logic in adaptNoArgs #3254
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
adaptNoArgs would do an implicit argument search only if the result type conformed to the expected type. scala#3252 shows that this needs to be revised.
Split into more manageable sub-methods
test performance please |
performance test scheduled: 1 job(s) in queue, 0 running. |
To review this it's best to look at the first commit in isolation, which is reasonably small. The rest is just moving code around. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/3254/ to see the changes. Benchmarks is based on merging with master (443bdb1) |
case wtp: ExprType => | ||
adaptInterpolated(tree.withType(wtp.resultType), pt) | ||
case wtp: ImplicitMethodType | ||
if constrainResult(wtp, followAlias(pt)) || !functionExpected => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the order of the checks matter here? I see that constrainResult
generates new constraints if it returns true
.
If the order does not matter, then it should be less expensive to first check !functionExpected
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good observation. The order does matter, actually. constrainResult
strenghtens the constraint if it succeeds so we want to always run it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
adaptNoArgs would do an implicit argument search only if the result type
conformed to the expected type. #3252 shows that this needs to be revised.