From cf1f290d9baa2c0425608142bd5bbc6ea1b22d71 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 10 Oct 2018 11:24:10 +0200 Subject: [PATCH 1/4] Fix #5224: Make implicit shadowing conform to the rules in the spec The previous version of implicit shadowing considered a nested reference as a shadow _only_ if it matched the prototype of the implicit search. This is not backed up by the spec, and is different to what scalac does. Arguably, the spec is right here: We should consider an implicit ineligible if it is not visible as a simple name because a nested reference has the same name, no matter what the type of the nested reference is. --- .../dotty/tools/dotc/typer/Implicits.scala | 38 +++++++++++-------- tests/neg/implicit-shadowing.scala | 19 +++++++++- 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index ebca515c08c0..b999a78677e8 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -16,6 +16,7 @@ import Mode.ImplicitsEnabled import NameOps._ import NameKinds.LazyImplicitName import Symbols._ +import Denotations._ import Types._ import Decorators._ import Names._ @@ -907,12 +908,6 @@ trait Implicits { self: Typer => /** The expected type for the searched implicit */ lazy val fullProto: Type = implicitProto(pt, identity) - lazy val funProto: Type = fullProto match { - case proto: ViewProto => - FunProto(untpd.TypedSplice(dummyTreeOfType(proto.argType)) :: Nil, proto.resultType)(self) - case proto => proto - } - /** The expected type where parameters and uninstantiated typevars are replaced by wildcard types */ val wildProto: Type = implicitProto(pt, wildApprox(_)) @@ -930,18 +925,31 @@ trait Implicits { self: Typer => untpd.Apply(untpd.TypedSplice(generated), untpd.TypedSplice(argument) :: Nil), pt, locked) val generated1 = adapt(generated, pt, locked) + lazy val shadowing = - typed(untpd.Ident(cand.implicitRef.implicitName) withPos pos.toSynthetic, funProto)( + typed(untpd.Ident(cand.implicitRef.implicitName) withPos pos.toSynthetic)( nestedContext().addMode(Mode.ImplicitShadowing).setExploreTyperState()) - def refSameAs(shadowing: Tree): Boolean = - ref.symbol == closureBody(shadowing).symbol || { - shadowing match { - case Trees.Select(qual, nme.apply) => refSameAs(qual) - case Trees.Apply(fn, _) => refSameAs(fn) - case Trees.TypeApply(fn, _) => refSameAs(fn) - case _ => false - } + + /** Is candidate reference the same as the `shadowing` reference? (i.e. + * no actual shadowoing occured). This is the case of the + * underlying symbol of the shadowing reference is the same as the + * symbol of the candidate reference, or if they have a common type owner. + * + * The second condition (same owner) is needed because the candidate reference + * and the potential shadowing reference are typechecked with different prototypes. + * so might yield different overloaded symbols. E.g. if the candidate reference + * is to an implicit conversion generated from an implicit class, the shadwoing + * reference could go to the companion object of that class instead. + */ + def refSameAs(shadowing: Tree): Boolean = { + def symMatches(sym: Symbol): Boolean = + sym == ref.symbol || sym.owner.isType && sym.owner == ref.symbol.owner + def denotMatches(d: Denotation): Boolean = d match { + case d: SingleDenotation => symMatches(d.symbol) + case d => d.hasAltWith(denotMatches(_)) } + denotMatches(closureBody(shadowing).denot) + } if (ctx.reporter.hasErrors) { ctx.reporter.removeBufferedMessages diff --git a/tests/neg/implicit-shadowing.scala b/tests/neg/implicit-shadowing.scala index ae33750eb72a..310925ae4911 100644 --- a/tests/neg/implicit-shadowing.scala +++ b/tests/neg/implicit-shadowing.scala @@ -5,8 +5,25 @@ object Test { def outer(implicit c: C) = { def f(c: C) = implicitly[C] // error: shadowing - def g(c: Int) = implicitly[C] // ok since type is different + def g(c: Int) = implicitly[C] // error: shadowing (even though type is different) f(new C) } + + class C1[X] + class C2[X] + + def f[T: C1] = { + def g[U: C2] = { + implicitly[C1[T]] // OK: no shadowing for evidence parameters + implicitly[C2[U]] + } + } + + def h[T]: implicit C1[T] => Unit = { + def g[U]: implicit C2[U] => Unit = { + implicitly[C1[T]] // OK: no shadowing for evidence parameters + implicitly[C2[U]] + } + } } From a5da78ad3ef4a33bd653c8b4bf78e827995755cd Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 10 Oct 2018 11:36:28 +0200 Subject: [PATCH 2/4] Fix typos in comment --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index b999a78677e8..383d0627bf51 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -925,20 +925,20 @@ trait Implicits { self: Typer => untpd.Apply(untpd.TypedSplice(generated), untpd.TypedSplice(argument) :: Nil), pt, locked) val generated1 = adapt(generated, pt, locked) - + lazy val shadowing = typed(untpd.Ident(cand.implicitRef.implicitName) withPos pos.toSynthetic)( nestedContext().addMode(Mode.ImplicitShadowing).setExploreTyperState()) /** Is candidate reference the same as the `shadowing` reference? (i.e. - * no actual shadowoing occured). This is the case of the + * no actual shadowing occured). This is the case if the * underlying symbol of the shadowing reference is the same as the * symbol of the candidate reference, or if they have a common type owner. * * The second condition (same owner) is needed because the candidate reference * and the potential shadowing reference are typechecked with different prototypes. * so might yield different overloaded symbols. E.g. if the candidate reference - * is to an implicit conversion generated from an implicit class, the shadwoing + * is to an implicit conversion generated from an implicit class, the shadowing * reference could go to the companion object of that class instead. */ def refSameAs(shadowing: Tree): Boolean = { From a2b323ea71b29520b79563b30818858eb644529b Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 10 Oct 2018 12:14:47 +0200 Subject: [PATCH 3/4] Give info about compiled files on every exception Helps to trouble-shoot the culprit during parallel testing --- compiler/src/dotty/tools/dotc/Driver.scala | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/Driver.scala b/compiler/src/dotty/tools/dotc/Driver.scala index e3d303807619..f319432199a6 100644 --- a/compiler/src/dotty/tools/dotc/Driver.scala +++ b/compiler/src/dotty/tools/dotc/Driver.scala @@ -34,6 +34,9 @@ class Driver { case ex: FatalError => ctx.error(ex.getMessage) // signals that we should fail compilation. ctx.reporter + case ex: Throwable => + println(s"$ex while compiling ${fileNames.mkString(", ")}") + throw ex } else ctx.reporter From 876fb32f830602e3175eb19bbc043b6079d3c449 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 10 Oct 2018 12:23:37 +0200 Subject: [PATCH 4/4] Do only a "typedUnadapted" when computing shadowing refs Since the type does not matter, we don't need to adapt it. --- .../dotty/tools/dotc/typer/Implicits.scala | 2 +- tests/run/i5224.check | 2 ++ tests/run/i5224.scala | 20 +++++++++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 tests/run/i5224.check create mode 100644 tests/run/i5224.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 383d0627bf51..5b8a0aa4d152 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -927,7 +927,7 @@ trait Implicits { self: Typer => val generated1 = adapt(generated, pt, locked) lazy val shadowing = - typed(untpd.Ident(cand.implicitRef.implicitName) withPos pos.toSynthetic)( + typedUnadapted(untpd.Ident(cand.implicitRef.implicitName) withPos pos.toSynthetic)( nestedContext().addMode(Mode.ImplicitShadowing).setExploreTyperState()) /** Is candidate reference the same as the `shadowing` reference? (i.e. diff --git a/tests/run/i5224.check b/tests/run/i5224.check new file mode 100644 index 000000000000..3cb9bd81d1bd --- /dev/null +++ b/tests/run/i5224.check @@ -0,0 +1,2 @@ +barInt +bar diff --git a/tests/run/i5224.scala b/tests/run/i5224.scala new file mode 100644 index 000000000000..9ac5ff23e2ab --- /dev/null +++ b/tests/run/i5224.scala @@ -0,0 +1,20 @@ +object Test extends App { + class Bar[T] + + implicit def barInt: Bar[Int] = { + println("barInt") + new Bar[Int] + } + implicit def bar[T]: Bar[T] = { + println("bar") + new Bar[T] + } + + implicitly[Bar[Int]] + + locally { + def barInt: Unit = ??? + + implicitly[Bar[Int]] + } +} \ No newline at end of file