-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #3246: Refine handling of postfix _ for non-functions #3300
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
@allanrenucci All patterns you showed in #3246 should now be handled without error under -language:Scala2 |
I'll try to revert the workaround in scalatest and see if I can compile with this patch |
class Test {
def foo(fun: () => Unit) = ???
def bar(fun: => Any) = foo(fun _)
} This test case still does not compile with Dotty |
Here's the error message I get (btw, for future reference, it would be good to add the actual error message to an issue or comment).
This looks OK to me. Why should it compile? I don't see how |
I agree it shouldn't compile. But then, should the following code compile? class Test {
def foo(fun: () => Unit) = ???
def bar(fun: => Any) = foo(() => fun)
} |
if (pt1.eq(AnyFunctionProto) && !defn.isFunctionClass(res.tpe.classSymbol)) { | ||
ctx.errorOrMigrationWarning(i"not a function: ${res.tpe}; cannot be followed by `_'", tree.pos) | ||
val nestedCtx = ctx.fresh.setNewTyperState() | ||
var res = typed(qual, pt1)(nestedCtx) |
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.
Can be a val
now
As a side effect of this PR, we now accept this code under class Test {
def foo(x: Int) = 1
val bar: () => Int = foo(1) _
} It is definitely an improvement as we used to emit a type error: -- [E007] Type Mismatch Error: tests/allan/Test.scala:3:26 ---------------------
3 | val bar: () => Int = foo(1) _
| ^^^^^^
| found: Int
| required: () => Int
|
one error found Now: -- Migration Warning: tests/allan/Test.scala:3:30 ------------------------------
3 | val bar: () => Int = foo(1) _
| ^^^^^^^^
| not a function: foo(1); cannot be followed by `_' |
class Test {
def foo = 1
val bar: () => Int = foo _
} -- Migration Warning: tests/allan/Test.scala:3:27 ------------------------------
3 | val bar: () => Int = foo _
| ^^^^^
| not a function: foo; cannot be followed by `_' I think the warning is misleading in this case. If foo is defined as |
class Test {
def foo(x: Int) = 1
val bar: () => Int = foo _
} This now crashes under
|
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 can confirm that we can now compile all the postfix _ in scalatest under scala2 mode.
Now this compile under scala2 mode:
class Test {
def foo(fun: () => Unit) = ???
def bar(fun: => Int) = foo(fun _) // accepted by scalac
foo(1 _) // rejected by scalac
}
You said previously that it was a bug in scalac. I guess it is fine to support this cases under scala2 mode (even if it is a bug) if we reject them under "normal" mode
@@ -0,0 +1,4 @@ | |||
class Test { |
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 think we should also compile it under scala2 mode, since it only crashed in this mode
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.
OK. Can you make the change and merge the PR when done? Thanks!
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.
Sure. Will do
Superseded by #3402 |
No description provided.