Fix #1959: infix type operators in the REPL #2162
Conversation
abeln
changed the title
|
This fixes #1959. |
|
Would you also kindly include a test to repl tests that would track that the issue stays fixed? |
|
@abeln Nice catch! Would you mind adding a REPL test? It's pretty easy, you just need to add a file in https://github.com/lampepfl/dotty/tree/master/tests/repl which contains the output of a REPL session. |
abeln
force-pushed
the
infix-op
branch
2 times, most recently
from
7a00823 to
f225d02
Compare
|
PTAL -- added repl test |
|
@DarkDimius oops, sorry -- I'd changed the PR title, but forgot the commit title. PTAL. |
| @@ -526,7 +526,7 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo { | |||
| case Function(args, body) => | |||
| this(this(x, args), body) | |||
| case InfixOp(left, op, right) => | |||
| this(this(x, left), right) | |||
| this(this(this(x, left), op), right) | |||
| case PostfixOp(od, op) => | |||
There was a problem hiding this comment.
I haven't checked how UntypedTreeAccumulator is used exactly, but if InfixOp folds over the op, then surely PostfixOp and PrefixOp should too?
There was a problem hiding this comment.
UntypedTreeAccumulator (and its cousing UntypedDeepFolder) is used only in the repl code (CompilingInterpreter.scala).
Changed {Post, Pre}fixOp as well.
PTAL.
Infix type operators were broken in the REPL. The REPL uses fold methods from the untpd package, but those were skipping the operator subtree when folding over an InfixOp. Fix the issue by taking the operator into account. Tested: 1) Verified that only the REPL code uses the modified fold method. 2) Added repl test.
|
@DarkDimius friendly ping :) |
| case PrefixOp(op, od) => | ||
| this(x, od) | ||
| this(this(x, op), od) |
There was a problem hiding this comment.
Would it be possible to expand the test to also test this change?
There was a problem hiding this comment.
Does Dotty have prefix type operators? If so, how are they used?
"Regular" prefix operators already worked in the REPL before this change: it's a different ast that doesn't end up using the code above.
This is the hypothetical test I'd add, but like I said it's not really exercising this PR.
scala> class B(val n: Int) {
def unary_! = new B(-n)
}
defined class B
scala> val x = new B(42)
val x: B = B@10e92f8f
scala> (!x).n
val res4: Int = -42
Thoughts?
There was a problem hiding this comment.
Yes, I agree. Let's get this in.
|
Thanks for the contribution! 🎉 |
Infix type operators were broken in the REPL.
The REPL uses fold methods from the untpd package,
but those were skipping the operator subtree when folding
over an InfixOp.
Fix the issue by taking the operator into account.
Tested:
fold method.
(used to fail with a 'can't find +' error):
scala> class +[A, B](a: A, B: B)
defined class +
scala> type IntAndString = Int + String
defined type alias IntAndString
scala> new IntAndString(42, "hello world")
val res0: +[Int, String] = $plus@6a472554