Scala 2/Dotty difference in implicit shadowing behaviour

[milessabin]

Scala 2 and Dotty compile the following example differently,

object Test {
  class Foo
  class Bar[T]
  class Baz[T] extends Bar[T]

  implicit def foo(implicit bar: Bar[Int]): Foo = ???
  implicit def barInt: Bar[Int] = ???
  implicit def bar[T]: Bar[T] = ???

  implicitly[Foo] // outer

  {
    def barInt: Unit = ???

    implicitly[Foo] // inner
  }
}

Dotty selects the barInt instance in both the outer and the inner cases. Scala 2 selects the barInt instance in the outer case, and the bar[T] instance in the inner case due to shadowing by the inner non-implicit definition of barInt.

It's not clear to me whether or not this is intentional; or a bug due to the inner context reusing the outer implicit cache without recognizing that the inner non-implicit definition requires the eligible set to be recomputed for the inner scope.

[milessabin]

A simpler example,

object Test {
  class Bar[T]
  class Baz[T] extends Bar[T]

  implicit def barInt: Bar[Int] = ???
  implicit def bar[T]: Bar[T] = ???

  implicitly[Bar[Int]] // outer

  {
    def barInt: Unit = ???

    implicitly[Bar[Int]] // inner
  }
}

[milessabin]

On balance I don't think I'd object to implicit shadowing being dropped altogether, but it's definitely a significant semantic change from Scala 2 ... existing code with break or change meaning.

FWIW, I think that supporting shadowing would most likely undermine the benefits of caching. When I attempted something similar for Scala 2 I was hamstrung by the fact that any difference in definitions in scope between two implicit resolution sites for the same type could result in the set of candidates needing to be recomputed, or at least the checks that the differences were irrelevant was not much less expensive than doing a full recomputation.

[smarter]

Pretty sure this is intentional yes, your code shouldn't suddenly break because you happened to use a name that was used by something you imported.

[smarter]

Keeping open since we should at least document that in http://dotty.epfl.ch/docs/reference/changed/implicit-resolution.html

[joroKr21]

Pretty sure this is intentional yes, your code shouldn't suddenly break because you happened to use a name that was used by something you imported.

What about import shadowing?

FWIW, I've used this trick a few times in Scala to work around poorly designed APIs. An example would be an API which requires the user to extend a class defining an implicit (such as a syntax extension) thus making it impossible for the user to opt out of the syntax. Most widely known example is scalatest's ===.

[smarter]

You can shadow imports in Dotty.

[milessabin]

your code shouldn't suddenly break because you happened to use a name that was used by something you imported.

I agree. Unfortunately Dotty isn't quite there yet ... there's some residual implicit shadowing logic which means that an inner definition will always dominate an outer definition, irrespective of its weight otherwise.

Consider,

object Test {
  class Foo[T]

  implicit def foo: Foo[Int] = ???

  {
    implicit def foo[T]: Foo[T] = ???

    implicitly[Foo[Int]] // compiles: outer foo is shadowed
  }

  {
    implicit def foo2[T]: Foo[T] = ???

    implicitly[Foo[Int]] // ambiguous
  }
}

Arguably both cases should be ambiguous.

[Blaisorblade]

@milessabin At least your latest example looks intentional, according to the blog post on implicit function types. This points starts at "There is one final tweak to make this all work", I'll just quote the final paragraph (see there for the motivation).

The problem is that parameters in implicit closures now have compiler-generated names, so the programmer cannot enforce the proper naming scheme to avoid all ambiguities. We fix the problem by introducing a new disambiguation rule which makes nested occurrences of an implicit take precedence over outer ones. This rule, which applies to all implicit parameters and implicit locals, is conceptually analogous to the rule that prefers implicits defined in companion objects of subclasses over those defined in companion objects of superclass. With that new disambiguation rule the example code above now compiles.

[milessabin]

@Blaisorblade the part about inner scopes taking priority over outer is intentional, and that's reflected in the ambiguous case: foo2 is +1 because it's in an inner scope and the outer foo is +1 because it's more specific, hence a tie, so ambiguous.

In the "successful" case the inner foo doesn't just get a priority boost due to being in an inner scope, it knocks the outer foo out of contention altogether due to shadowing, despite being less specific.

I think the motivation for having scope depth affect priority, namely that "the programmer cannot enforce the proper naming scheme to avoid all ambiguities" applies across the board with shadowing, so I'd like to see both cases treated as ambiguous.

[odersky]

I think the spec is right here and dotty is wrong. See the comment in #5227.

[LPTK]

@milessabin I feel like there is a difference between:

• 'unhygienic' shadowing, which is when names from different files interact with each other in unpredictable ways, which is definitely undesirable – it means people need to invent crazy names for their implicits just to avoid unintentional name clashes in different files/projects

• local scope shadowing, which can be more naturally reasoned about and is more obvious (does not involve non-local reasoning)

The latter seems okay to me. It is clear that in your example the local binding foo is shadowed. Besides, in general what should the elaborated code look like if we disregarded local-scope shadowing? We can't refer to a shadowed local binding:

{
  implicit def foo: Foo[Int] = ???
  
  {
    def foo: Int = 1

    implicitly[Foo[Int]] // what should this desugar to?
  }
}

...the compiler would have to insert some renaming logic, but then it means a user cannot always manually elaborate an implicit argument if desired, without having to make other changes to the program (to get around possible shadowing of local implicit identifiers).

[milessabin]

the compiler would have to insert some renaming logic, but then it means a user cannot always manually elaborate an implicit argument if desired, without having to make other changes to the program (to get around possible shadowing of local implicit identifiers)

The user would also have to manually elaborate the renaming ... I don't think I see that as a significant problem here.