-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Type ascribe arguments of bottom type to inline functions #11917
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
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.
Otherwise, LGTM
true | ||
computeParamBindings( | ||
tp.resultType, targs.drop(tp.paramNames.length), argss, | ||
paramSubst.andThen(_.substParams(tp, targs.map(_.tpe.stripTypeVar)))) |
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.
It seems the usage of tp
will disregard the prefix -- maybe methPart.tpe.widen
is more accurate? Meanwhile, the current fix does not handle dependent types. Will it be simpler to extract the types from the fun part of Apply trees instead of recomputing them?
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. Yes, that should be fixed.
def paramTypess(call: Tree, acc: List[List[Type]]): List[List[Type]] = call match | ||
case Apply(fn, args) => | ||
fn.tpe.widen.match | ||
case mt: MethodType => paramTypess(fn, mt.instantiateParamInfos(args.tpes) :: acc) |
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.
I'm wondering will mt.paramInfos
work here?
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.
No, because parameter types can depend on parameters in the same clause.
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.
That's tested in test4, which failed before when I used paramInfos.
Fixes #11864
Fixes #8612