Fix #5224: Make implicit shadowing conform to the rules in the spec

[odersky]

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.

[milessabin]

Should a nested non-implicit definition shadow an implicit definition, ie. should the following not compile,

object Test {
  class Foo
  implicit def foo: Foo = ???
  {
    def foo = ???
    implicitly[Foo]  // outer foo is shadowed
  }
}

[odersky]

object Test {
  class Foo
  implicit def foo: Foo = ???

  {
    def foo = ???
    implicitly[Foo]  // outer foo is shadowed
  }

This should not compile, since the rules say "implicits are eligible if they can be referred to by a simple identifier". That's not the case in this example.

[milessabin]

FWIW, it compiles both with Dotty prior to this PR and Scala 2.13.x.

[milessabin]

I'd like to play around with this a bit ... I was digging around here because I had a hunch that shadowing might interact badly with your implicit caching strategy. I wasn't able to come up with a problematic case for caching because, prior to this PR, eligible implicits more or less only increased monotonically with more nested scopes. Now that shadowing can knock out arbitrary implicit definitions in outer scopes I think it might be possible to trick the caching mechanism into miscompiling.

[milessabin]

@odersky that example still compiles after this PR.

The issue is in computeEligible ... shadowers are only the inner contexts ownEligible which must both be implicit and match the expected type.

[milessabin]

Correction, the problem seems to be here: https://github.com/lampepfl/dotty/blob/876fb32f830602e3175eb19bbc043b6079d3c449/compiler/src/dotty/tools/dotc/typer/Implicits.scala#L963-L970

Second time though, contextual is false and the candidate which should have been shadowed is allowed through.

[milessabin]

Is the issue that contextual implicits are retained in the cache and then found via a cache hit when only the implicit scope should be considered?

[odersky]

Second time though, contextual is false and the candidate which should have been shadowed is allowed through.

That's also as required. Implicits in the implicit scope cannot be shadowed. The rules are:

Contextual implicit: Any implicit value that can be referred to with a simple name qualifies
Type scope implicit: All implicit values in the implicit scope of the required type qualify.

Shadowing comes up only indirectly, to check that any contextual implicit that was found actually qualifies.

[milessabin]

That's also as required. Implicits in the implicit scope cannot be shadowed.

Understood. But there's no implicit scope in play here.

[odersky]

Understood. But there's no implicit scope in play here.

I think there is: The full type is Test.Foo. Since foo is in Test, it's in the implicit scope.

(Aside: We should try whether we can reduce this aspect of type prefixes contributing to the implicit scope. In the past there have been some abuses where implicits were placed in package objects which made their implicit scope surprisingly large. So it might be good if we could at least avoid packages as parts of implicit scopes.)