SI-9745 typedFunction keeps x => f(x) #6007
Conversation
To type `x => f(x)`, `typedFunction` uses the expected type and `f` to type `x`. Previously, it discarded the result of typechecking `f`, which was a problem in the case of a `g(e)` where `e` is "erroneous" but rewriteable to an expr that checks, e.g., `g(i += 1)`. This commit keeps the rewritten `g(i = i + 1)` instead of trying to recover the erroneous tree.
| // we can compare arguments and parameters by name because there cannot be a binder between | ||
| // the function's valdefs and the Apply's arguments | ||
| case Apply(meth, args) if (vparams corresponds args) { case (p, Ident(name)) => p.name == name case _ => false } => | ||
| // We're looking for a method (as indicated by FUNmode in the silent typed below), | ||
| // so let's make sure our expected type is a MethodType | ||
| val methArgs = NoSymbol.newSyntheticValueParams(argpts map { case NoType => WildcardType case tp => tp }) | ||
|
|
||
| val result = silent(_.typed(meth, mode.forFunMode, MethodType(methArgs, respt))) |
There was a problem hiding this comment.
Let's explore keeping the silent and a variation of throwing in a .duplicate -> silent(_.typed(meth.duplicate, mode.forFunMode, MethodType(methArgs, respt))). That fixed the issue for me as well. I'm hesitant to drop the silent, but grant that duplication is costly.
There was a problem hiding this comment.
IIRC there was a previous PR that duplicates? For some reason, it seemed simpler just to accept the unsilent judgment; but I don't spend all day with the code, and also I don't remember what I did last summer. Maybe this is an opportunity to try some jmh-driven development.
|
I'm going to defer until M4, because I'm dubious on dropping |
|
@adriaanm Sorry I haven't had a chance to get back to this. It doesn't take much to blow my lunchtime budget for FOSS. In 2018, though, I hope to get better at contributing review time, which is probably more useful than marginal repairs. (But please don't quote me; I'll probably delete this resolution next December.) |
|
I'm pushing this to the RC, but I'm open to a refined fix either before (M5) or after (2.13.x) |
|
needs rebase |
test case taken from original PR (scala#6007)
|
Adopting as #7316 |
|
@adriaanm I have a few other jobs around the house I've been meaning to tackle, if you have time. |
|
What’s your travel cost policy?
…On Fri, Oct 5, 2018 at 17:43 som-snytt ***@***.***> wrote:
@adriaanm <https://github.com/adriaanm> I have a few other jobs around
the house I've been meaning to tackle, if you have time.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6007 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAFjy9tDBei9Elk-Xg-dz287yZxYRJuCks5uh34IgaJpZM4OjAzx>
.
|
Do more for typedFunction of eta-expanded method: keep typed method selection (fix scala/bug#9745, follow up for scala#6007) Test case taken from original PR.
Do more for typedFunction of eta-expanded method: keep typed method selection (fix scala/bug#9745, follow up for scala#6007) Test case taken from original PR. Backported from 64d4c24
Do more for typedFunction of eta-expanded method: keep typed method selection (fix scala/bug#9745, follow up for scala#6007) Test case taken from original PR. Backported from 64d4c24
To type
x => f(x),typedFunctionuses the expectedtype and
fto typex. Previously, it discarded theresult of typechecking
f, which was a problem in thecase of a
g(e)whereeis "erroneous" but rewriteableto an expr that checks, e.g.,
g(i += 1).This commit keeps the rewritten
g(i = i + 1)insteadof trying to recover the erroneous tree.