Skip to content

Allow overloading a method with a signature which matches before erasure but not after #597

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.

Sign up for GitHub

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

Jump to bottom
Closed
smarter opened this issue May 22, 2015 · 4 comments
Closed

Allow overloading a method with a signature which matches before erasure but not after #597

smarter opened this issue May 22, 2015 · 4 comments

Comments

@smarter
Copy link
Member

smarter commented May 22, 2015

This works in scala but not in dotty:

trait A
trait B

class Test {
  def foo(x: List[A]): Function1[A, A] = ???
  def foo(x: List[B]): Function2[B, B, B] = ???
}

It fails with:

[error] eras.scala:6: error: method foo is already defined as method foo: (x: scala.collection.immutable.List[A])A => A
[error]  (both definitions have the same erased type signature)
[error]   def foo(x: List[B]): Function2[B, B, B] = ???
[error]         ^
[error] one error found
j

Even though they don't have the same erased type signature.

The problem is that the following code in Checking#checkNoDoubleDefs:

if (decl.signature matches other.signature) {

Is run before erasure, and matches is defined as:

  /** The meaning of `matches` depends on the phase. If types are not erased,
   *  it means `sameParams`. Once types are erased, it means `==`, comparing parameter as
   *  well as result type parts.
   */
  final def matches(that: Signature)(implicit ctx: Context) =
    if (ctx.erasedTypes) equals(that) else sameParams(that)

Simply changing the code in Checking#checkNoDoubleDefs is probably not enough to fix that since the compiler complains:

warning: cannot merge (x: scala.collection.immutable.List[B])(B, B) => B with (x: scala.collection.immutable.List[A])A => A as members of one type; keeping only (x: scala.collection.immutable.List[B])(B, B) => B
warning: cannot merge (x: scala.collection.immutable.List[B])(B, B) => B with (x: scala.collection.immutable.List[A])A => A as members of one type; keeping only (x: scala.collection.immutable.List[B])(B, B) => B

And changing Signature#matches to always match the whole signature and not just the params breaks everything in a way I don't understand:

[info] applied type mismatch: TypeRef(TermRef(TermRef(TermRef(TermRef(TermRef(ThisType(TypeRef(NoPrefix,<root>)),scala),reflect),runtime),package),universe),TypeTag) List(TypeRef(NoPrefix,dotty$DottyPredef$typeTag$$T)), typeParams = List(), tsym = val <none>#1
[info] precomplete decls = 
[error] java.util.NoSuchElementException: head of empty list
[error]         at scala.collection.immutable.Nil$.head(List.scala:420)
[error]         at scala.collection.immutable.Nil$.head(List.scala:417)
[error]         at dotty.tools.dotc.core.TypeApplications$$anonfun$appliedTo$extension0$1.matchParams$1(TypeApplications.scala:146)
[error]         at dotty.tools.dotc.core.TypeApplications$$anonfun$appliedTo$extension0$1.instantiate$1(TypeApplications.scala:173)
[error]         at dotty.tools.dotc.core.TypeApplications$$anonfun$appliedTo$extension0$1.apply(TypeApplications.scala:190)
[error]         at dotty.tools.dotc.core.TypeApplications$$anonfun$appliedTo$extension0$1.apply(TypeApplications.scala:139)
[error]         at dotty.tools.dotc.util.Stats$.track(Stats.scala:35)
[error]         at dotty.tools.dotc.core.TypeApplications$.appliedTo$extension0(TypeApplications.scala:139)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler.readType(Scala2Unpickler.scala:679)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler$$anonfun$readTypeRef$1.apply(Scala2Unpickler.scala:795)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler$$anonfun$readTypeRef$1.apply(Scala2Unpickler.scala:795)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler.atReadPos(Scala2Unpickler.scala:326)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler.at(Scala2Unpickler.scala:316)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler.readTypeRef(Scala2Unpickler.scala:795)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler.readType(Scala2Unpickler.scala:710)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler$$anonfun$readTypeRef$1.apply(Scala2Unpickler.scala:795)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler$$anonfun$readTypeRef$1.apply(Scala2Unpickler.scala:795)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler.atReadPos(Scala2Unpickler.scala:326)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler.at(Scala2Unpickler.scala:316)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler.readTypeRef(Scala2Unpickler.scala:795)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler.readType(Scala2Unpickler.scala:710)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler$LocalUnpickler$$anonfun$9.apply(Scala2Unpickler.scala:537)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler$LocalUnpickler$$anonfun$9.apply(Scala2Unpickler.scala:537)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler.atReadPos(Scala2Unpickler.scala:326)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler.at(Scala2Unpickler.scala:316)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler$LocalUnpickler.dotty$tools$dotc$core$unpickleScala2$Scala2Unpickler$LocalUnpickler$$parseToCompletion$1(Scala2Unpickler.scala:537)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler$LocalUnpickler$$anonfun$complete$1.apply$mcV$sp(Scala2Unpickler.scala:566)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler$LocalUnpickler$$anonfun$complete$1.apply(Scala2Unpickler.scala:566)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler$LocalUnpickler$$anonfun$complete$1.apply(Scala2Unpickler.scala:566)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler.atReadPos(Scala2Unpickler.scala:326)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler$LocalUnpickler.complete(Scala2Unpickler.scala:566)
[error]         at dotty.tools.dotc.core.SymDenotations$SymDenotation.completeFrom(SymDenotations.scala:161)
[error]         at dotty.tools.dotc.core.SymDenotations$SymDenotation.info(SymDenotations.scala:143)
[error]         at dotty.tools.dotc.transform.TreeTransforms$AnnotationTransformer$class.transform(TreeTransform.scala:183)
[error]         at dotty.tools.dotc.transform.FirstTransform.transform(FirstTransform.scala:30)
[error]         at dotty.tools.dotc.core.Denotations$SingleDenotation.currentIfExists(Denotations.scala:580)
[error]         at dotty.tools.dotc.core.Denotations$SingleDenotation.current(Denotations.scala:616)
[error]         at dotty.tools.dotc.core.Denotations$SingleDenotation.currentIfExists(Denotations.scala:598)
[error]         at dotty.tools.dotc.core.Denotations$SingleDenotation.current(Denotations.scala:616)
[error]         at dotty.tools.dotc.core.Symbols$Symbol.denot(Symbols.scala:394)
[error]         at dotty.tools.dotc.core.Scopes$Scope.denotsNamed(Scopes.scala:119)
[error]         at dotty.tools.dotc.core.SymDenotations$ClassDenotation.membersNamed(SymDenotations.scala:1412)
[error]         at dotty.tools.dotc.core.SymDenotations$ClassDenotation.findMember(SymDenotations.scala:1465)
[error]         at dotty.tools.dotc.core.Types$Type.go$1(Types.scala:441)
[error]         at dotty.tools.dotc.core.Types$Type.findMember(Types.scala:491)
[error]         at dotty.tools.dotc.core.Types$Type$$anonfun$member$1.apply(Types.scala:407)
[error]         at dotty.tools.dotc.core.Types$Type$$anonfun$member$1.apply(Types.scala:407)
[error]         at dotty.tools.dotc.util.Stats$.track(Stats.scala:35)
[error]         at dotty.tools.dotc.core.Types$Type.member(Types.scala:406)
[error]         at dotty.tools.dotc.core.Symbols$class.requiredMethod(Symbols.scala:362)
[error]         at dotty.tools.dotc.core.Contexts$Context.requiredMethod(Contexts.scala:51)
[error]         at dotty.tools.dotc.transform.ClassTags.prepareForUnit(ClassTags.scala:36)
[error]         at dotty.tools.dotc.transform.TreeTransforms$TreeTransformer$$anonfun$34.apply(TreeTransform.scala:549)
[error]         at dotty.tools.dotc.transform.TreeTransforms$TreeTransformer$$anonfun$34.apply(TreeTransform.scala:549)
[error]         at dotty.tools.dotc.transform.TreeTransforms$TreeTransformer.mutateTransformers(TreeTransform.scala:501)
[error]         at dotty.tools.dotc.transform.TreeTransforms$TreeTransformer.macroTransform(TreeTransform.scala:561)
[error]         at dotty.tools.dotc.transform.TreeTransforms$TreeTransformer.run(TreeTransform.scala:480)
[error]         at dotty.tools.dotc.core.Phases$Phase$$anonfun$runOn$1.apply(Phases.scala:270)
[error]         at dotty.tools.dotc.core.Phases$Phase$$anonfun$runOn$1.apply(Phases.scala:268)
[error]         at scala.collection.immutable.List.map(List.scala:273)
[error]         at dotty.tools.dotc.core.Phases$Phase$class.runOn(Phases.scala:268)
[error]         at dotty.tools.dotc.transform.TreeTransforms$TreeTransformer.runOn(TreeTransform.scala:474)
[error]         at dotty.tools.dotc.Run$$anonfun$compileUnits$1$$anonfun$apply$mcV$sp$1.apply(Run.scala:59)
[error]         at dotty.tools.dotc.Run$$anonfun$compileUnits$1$$anonfun$apply$mcV$sp$1.apply(Run.scala:56)
[error]         at scala.collection.IndexedSeqOptimized$class.foreach(IndexedSeqOptimized.scala:33)
[error]         at scala.collection.mutable.ArrayOps$ofRef.foreach(ArrayOps.scala:186)
[error]         at dotty.tools.dotc.Run$$anonfun$compileUnits$1.apply$mcV$sp(Run.scala:56)
[error]         at dotty.tools.dotc.Run$$anonfun$compileUnits$1.apply(Run.scala:52)
[error]         at dotty.tools.dotc.Run$$anonfun$compileUnits$1.apply(Run.scala:52)
[error]         at dotty.tools.dotc.util.Stats$.monitorHeartBeat(Stats.scala:68)
[error]         at dotty.tools.dotc.Run.compileUnits(Run.scala:52)
[error]         at dotty.tools.dotc.Run.compileSources(Run.scala:49)
[error]         at dotty.tools.dotc.Run.compile(Run.scala:33)
[error]         at dotty.tools.dotc.Driver.doCompile(Driver.scala:20)
[error]         at dotty.tools.dotc.Main$.doCompile(Main.scala:26)
[error]         at dotty.tools.dotc.Driver.process(Driver.scala:31)
[error]         at dotty.tools.dotc.Driver.main(Driver.scala:40)
[error]         at dotty.tools.dotc.Main.main(Main.scala)
[info] findMember exception for ClassInfo(ThisType(TypeRef(NoPrefix,dotty)), module class DottyPredef$) member typeTag
[info] exception occurred while compiling try/eras.scala
[error] Exception in thread "main" dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler$BadSignature: error reading Scala signature of class DottyPredef from /home/smarter/opt/dotty/target/scala-2.11/dotty_2.11-0.1-SNAPSHOT.jar(dotty/DottyPredef.class):
[error] error occurred at position 1181: a runtime exception occurred: java.util.NoSuchElementException: head of empty list
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler.errorBadSignature(Scala2Unpickler.scala:182)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler.handleRuntimeException(Scala2Unpickler.scala:191)
[error]         at dotty.tools.dotc.core.unpickleScala2.Scala2Unpickler$LocalUnpickler.complete(Scala2Unpickler.scala:568)
[error]         at dotty.tools.dotc.core.SymDenotations$SymDenotation.completeFrom(SymDenotations.scala:161)
[error]         at dotty.tools.dotc.core.SymDenotations$SymDenotation.info(SymDenotations.scala:143)
[error]         at dotty.tools.dotc.transform.TreeTransforms$AnnotationTransformer$class.transform(TreeTransform.scala:183)
[error]         at dotty.tools.dotc.transform.FirstTransform.transform(FirstTransform.scala:30)
[error]         at dotty.tools.dotc.core.Denotations$SingleDenotation.currentIfExists(Denotations.scala:580)
[error]         at dotty.tools.dotc.core.Denotations$SingleDenotation.current(Denotations.scala:616)
[error]         at dotty.tools.dotc.core.Denotations$SingleDenotation.currentIfExists(Denotations.scala:598)
[error]         at dotty.tools.dotc.core.Denotations$SingleDenotation.current(Denotations.scala:616)
[error]         at dotty.tools.dotc.core.Symbols$Symbol.denot(Symbols.scala:394)
[error]         at dotty.tools.dotc.core.Scopes$Scope.denotsNamed(Scopes.scala:119)
[error]         at dotty.tools.dotc.core.SymDenotations$ClassDenotation.membersNamed(SymDenotations.scala:1412)
[error]         at dotty.tools.dotc.core.SymDenotations$ClassDenotation.findMember(SymDenotations.scala:1465)
[error]         at dotty.tools.dotc.core.Types$Type.go$1(Types.scala:441)
[error]         at dotty.tools.dotc.core.Types$Type.findMember(Types.scala:491)
[error]         at dotty.tools.dotc.core.Types$Type$$anonfun$member$1.apply(Types.scala:407)
[error]         at dotty.tools.dotc.core.Types$Type$$anonfun$member$1.apply(Types.scala:407)
[error]         at dotty.tools.dotc.util.Stats$.track(Stats.scala:35)
[error]         at dotty.tools.dotc.core.Types$Type.member(Types.scala:406)
[error]         at dotty.tools.dotc.core.Symbols$class.requiredMethod(Symbols.scala:362)
[error]         at dotty.tools.dotc.core.Contexts$Context.requiredMethod(Contexts.scala:51)
[error]         at dotty.tools.dotc.transform.ClassTags.prepareForUnit(ClassTags.scala:36)
[error]         at dotty.tools.dotc.transform.TreeTransforms$TreeTransformer$$anonfun$34.apply(TreeTransform.scala:549)
[error]         at dotty.tools.dotc.transform.TreeTransforms$TreeTransformer$$anonfun$34.apply(TreeTransform.scala:549)
[error]         at dotty.tools.dotc.transform.TreeTransforms$TreeTransformer.mutateTransformers(TreeTransform.scala:501)
[error]         at dotty.tools.dotc.transform.TreeTransforms$TreeTransformer.macroTransform(TreeTransform.scala:561)
[error]         at dotty.tools.dotc.transform.TreeTransforms$TreeTransformer.run(TreeTransform.scala:480)
[error]         at dotty.tools.dotc.core.Phases$Phase$$anonfun$runOn$1.apply(Phases.scala:270)
[error]         at dotty.tools.dotc.core.Phases$Phase$$anonfun$runOn$1.apply(Phases.scala:268)
[error]         at scala.collection.immutable.List.map(List.scala:273)
[error]         at dotty.tools.dotc.core.Phases$Phase$class.runOn(Phases.scala:268)
[error]         at dotty.tools.dotc.transform.TreeTransforms$TreeTransformer.runOn(TreeTransform.scala:474)
[error]         at dotty.tools.dotc.Run$$anonfun$compileUnits$1$$anonfun$apply$mcV$sp$1.apply(Run.scala:59)
[error]         at dotty.tools.dotc.Run$$anonfun$compileUnits$1$$anonfun$apply$mcV$sp$1.apply(Run.scala:56)
[error]         at scala.collection.IndexedSeqOptimized$class.foreach(IndexedSeqOptimized.scala:33)
[error]         at scala.collection.mutable.ArrayOps$ofRef.foreach(ArrayOps.scala:186)
[error]         at dotty.tools.dotc.Run$$anonfun$compileUnits$1.apply$mcV$sp(Run.scala:56)
[error]         at dotty.tools.dotc.Run$$anonfun$compileUnits$1.apply(Run.scala:52)
[error]         at dotty.tools.dotc.Run$$anonfun$compileUnits$1.apply(Run.scala:52)
[error]         at dotty.tools.dotc.util.Stats$.monitorHeartBeat(Stats.scala:68)
[error]         at dotty.tools.dotc.Run.compileUnits(Run.scala:52)
[error]         at dotty.tools.dotc.Run.compileSources(Run.scala:49)
[error]         at dotty.tools.dotc.Run.compile(Run.scala:33)
[error]         at dotty.tools.dotc.Driver.doCompile(Driver.scala:20)
[error]         at dotty.tools.dotc.Main$.doCompile(Main.scala:26)
[error]         at dotty.tools.dotc.Driver.process(Driver.scala:31)
[error]         at dotty.tools.dotc.Driver.main(Driver.scala:40)
[error]         at dotty.tools.dotc.Main.main(Main.scala)

We could also choose to not support this kind of overloading, but it is needed to compile Function.scala:

  /** Uncurrying for functions of arity 2. This transforms a unary function
   *  returning another unary function into a function of arity 2.
   */
  def uncurried[a1, a2, b](f: a1 => a2 => b): (a1, a2) => b = {
    (x1, x2) => f(x1)(x2)
  }

  /** Uncurrying for functions of arity 3.
   */
  def uncurried[a1, a2, a3, b](f: a1 => a2 => a3 => b): (a1, a2, a3) => b = {
    (x1, x2, x3) => f(x1)(x2)(x3)
  }

  ...

@odersky : What do you think?
@smarter smarter changed the title Allow overriding a method with a signature which matches before erasure but not after Allow overloading a method with a signature which matches before erasure but not after May 22, 2015
@DarkDimius
Copy link
Contributor

Even though they don't have the same erased type signature.

They do, given current defition of

final def matches(that: Signature)(implicit ctx: Context) =
    if (ctx.erasedTypes) equals(that) else sameParams(that)

AFAIK that's intentional. We stopped assuming that return type is part of signature for duplicate detection

@smarter
Copy link
Member Author

smarter commented May 22, 2015

No, the erased("if ctx.erasedTypes") signature is different (because it looks at the return value), the non-erased signature is the same (because it only look at the parameters). But Checking#checkNoDoubleDefs is run before erasure.

@DarkDimius
Copy link
Contributor

@smarter after erasure we need to treat those methods as different, due to bridge generation for example.
But before erasure, we intentionally disallow them, users should not define such methods.

@odersky
Copy link
Contributor

odersky commented May 23, 2015

Indeed several parts in the frontend rely on this behavior of signatures. E.g. when looking things up, we merge ethods (decide they override instead of overload) based on their parameter signature. Since it is not a big loss to disallow these things I beleive we should do that. @smarter can you do a PR that changes the error message from (both definitions have the same erased type signature) to (the definitions have matching type signatures)?

DarkDimius added a commit that referenced this issue May 23, 2015
@DarkDimius
Merge pull request #600 from smarter/fix/overriden-message
14051fb 
Fix #597: Improve double def error message
@Blaisorblade Blaisorblade mentioned this issue Mar 8, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@smarter @odersky @DarkDimius