Fix #4564: Invalidate clashing case class methods #4813
Conversation
Invalidate a compiler generated case class method that clashes with a user-defined method in the same scope with a matching type. Invalidation is done by overwriting its info on completion with `NoType` and eliminating the method altogether during type checking. Methods potentially affected are `apply`, `unapply`, selectors `_1`, `_2`, ... `copy`, and `copy`'s default getters.
|
See also scala/scala#5816. |
There was a problem hiding this comment.
If we invalidate an _X method we should also drop the isDefined method, otherwise pattern matching may start behaving weirdly:
case class Foo(x: Int) {
def _1: Int = 42
}
object Test {
def main(args: Array[String]): Unit = {
Foo(1) match {
case Foo(x) =>
println(x) // prints 1 with scalac, 42 with dotty and this PR
}
}
}
|
@smarter I believe it's probably better if we exclude selectors from the scheme altogether. |
| def typedDefDef(ddef: untpd.DefDef, sym: Symbol)(implicit ctx: Context): Tree = track("typedDefDef") { | ||
| if (!sym.info.exists) { // it's a discarded synthetic case class method, drop it | ||
| assert(sym.is(Synthetic) && desugar.isDesugaredCaseClassMethodName(sym.name)) | ||
| sym.owner.info.decls.openForMutations.unlink(sym) |
There was a problem hiding this comment.
Neat! We should consider dropping here, instead of in the completer, for scalac too.
There was a problem hiding this comment.
It works because dotc is quite good at deciding that symbols with NoType as info don't really exist. (c.f. reallyExsists and its references). I am not sure what the state for scalac is here.
| @@ -0,0 +1,63 @@ | |||
| case class A(x: Int) { | |||
There was a problem hiding this comment.
There's also a negative version -- would be good to include.
|
PS: A pleasure to read the code! |
If we allow user-defined selectors _1, _2, and so on, we break the link with pattern matching.
If typing with a compiler-inserted apply gives an error, but the tree `t` in question does have an `apply` method, continue reporting errors for `t.apply` instead of `t`. This is likely more informative than simply pretending there was no `apply` to insert.
|
Added the neg test. This required some more changes in Typer to give better error messages similar to what's done in scala/scala. |
Invalidate a compiler generated case class method that clashes
with a user-defined method in the same scope with a matching type.
Invalidation is done by overwriting its info on completion with
NoTypeandeliminating the method altogether during type checking.
Methods potentially affected are
apply,unapply,copy, andcopy's default getters.