Skip to content
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.

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

Implicit Lookup fails for implicit view on stable nested type. #4947

Closed
scabug opened this issue Aug 28, 2011 · 5 comments · Fixed by scala/scala#6010
Closed

Implicit Lookup fails for implicit view on stable nested type. #4947

scabug opened this issue Aug 28, 2011 · 5 comments · Fixed by scala/scala#6010

Comments

@scabug
Copy link

scabug commented Aug 28, 2011

Derived from #4225.
Let me open this ticket, which I think important.
Without type-ascription, implicit lookup fails.

class DependentImplicitTezt {

    trait Bridge

    class Outer {
        class Inner extends Bridge

        object Inner {
            implicit def fromOther(b: Bridge): Inner = throw new Error("todo")
        }

        def run(x: Inner) = throw new Error("todo")
    }

    val o1 = new Outer
    val o2 = new Outer
    val i1 = new o1.Inner
    val i2 = new o2.Inner

    def doesntCompile {
        o1.run(i2) // no
    }

    def workaround1 {
        o1.run(i2: Bridge) // ok
    }

    def workaround2 {
        import o1.Inner.fromOther
        o1.run(i2) // ok
    }
}
@scabug
Copy link
Author

scabug commented Aug 28, 2011

Imported From: https://issues.scala-lang.org/browse/SI-4947?orig=1
Reporter: @okomok
Affected Versions: 2.9.0-1, 2.9.1
See #4225, #5340

@scabug
Copy link
Author

scabug commented May 20, 2012

@retronym said:
Seems similar to #5340

@scabug
Copy link
Author

scabug commented May 21, 2012

@retronym said (edited on May 21, 2012 5:44:00 AM UTC):
Yep, it's the same limitation as #5340

@scabug scabug closed this as completed May 21, 2012
@scabug
Copy link
Author

scabug commented May 21, 2012

@retronym said (edited on May 21, 2012 5:46:11 AM UTC):
Linkng to #4225, from whence this ticket came.

@milessabin
Copy link

PR: scala/scala#6010

retronym pushed a commit to retronym/scala that referenced this issue Jan 13, 2018
When the implicit scope is constructed an initial pruning phase attempts to
discard implicit members which it is believed would later be rejected as
ambiguous. Members of a companion are discarded if they can be reached via
more than one prefix, on the assumption that being visible via two or more
paths means that the multiple values must conflict with one another. However,
this does not take into account the possibility that the implicit members
might depend on the prefix in a way which renders them non-ambiguous. This
scenario is illustrated in scala/bug#4947 and
scala/bug#5340. This PR address that by only pruning
implicits which are independent of the prefix.

Whilst this PR tries to stay as close as possible to the current behaviour, I
believe that it would be desirable to drop the early pruning of even the
prefix-independent implicits. If we do that then we get improved error
messages reporting ambiguity rather the ambiguous members being silently
ignored which leads to otherwise unexplained failures of implicit resolution.
For instance in the case of `test/files/neg/t5340.scala` which currently
reports,

```
t5340.scala:15: error: type mismatch;
 found   : Quux[MyApp.r.E,MyApp.s.E]
 required: Int
  (new Quux[r.E, s.E]): Int // fails due to pre-stripping implicits which
   ^
one error found
```

we would get the following report,

```
t5340.scala:15: error: type mismatch;
 found   : Quux[MyApp.r.E,MyApp.s.E]
 required: Int
Note that implicit conversions are not applicable because they are ambiguous:
 both method conv in object E of type [T, U](b: Quux[T,U])Int
 and method conv in object E of type [T, U](b: Quux[T,U])Int
 are possible conversion functions from Quux[MyApp.r.E,MyApp.s.E] to Int
  (new Quux[r.E, s.E]): Int // fails due to pre-stripping implicits which
   ^
one error found
```

Which is clearly more informative (even if also a little baffling). I think
that ambiguity via multiple paths is sufficiently rare that the performance
benefits of this early pruning are likely to minimal and not enough to
outweigh improved error messages.

Because this change can result in a symbol _legitimately_ being revisited, via
a different prefix, we need to change the way that the a pruned path is
represented. Instead of using empty lists, as introduced in scala#5951 (which fixes
scala/bug#10081) we use explicit sentinels to mark
that a symbol has been visited at a given prefix yielding no additional
implicits. Testing this yielded scala/bug#10425
which appears to be a regression relative to 2.12.x. That's also fixed in this
PR.
milessabin added a commit to milessabin/scala that referenced this issue Jan 13, 2018
When the implicit scope is constructed an initial pruning phase attempts to
discard implicit members which it is believed would later be rejected as
ambiguous. Members of a companion are discarded if they can be reached via
more than one prefix, on the assumption that being visible via two or more
paths means that the multiple values must conflict with one another. However,
this does not take into account the possibility that the implicit members
might depend on the prefix in a way which renders them non-ambiguous. This
scenario is illustrated in scala/bug#4947 and
scala/bug#5340. This PR address that by only pruning
implicits which are independent of the prefix.

Whilst this PR tries to stay as close as possible to the current behaviour, I
believe that it would be desirable to drop the early pruning of even the
prefix-independent implicits. If we do that then we get improved error
messages reporting ambiguity rather the ambiguous members being silently
ignored which leads to otherwise unexplained failures of implicit resolution.
For instance in the case of `test/files/neg/t5340.scala` which currently
reports,

```
t5340.scala:15: error: type mismatch;
 found   : Quux[MyApp.r.E,MyApp.s.E]
 required: Int
  (new Quux[r.E, s.E]): Int // fails due to pre-stripping implicits which
   ^
one error found
```

we would get the following report,

```
t5340.scala:15: error: type mismatch;
 found   : Quux[MyApp.r.E,MyApp.s.E]
 required: Int
Note that implicit conversions are not applicable because they are ambiguous:
 both method conv in object E of type [T, U](b: Quux[T,U])Int
 and method conv in object E of type [T, U](b: Quux[T,U])Int
 are possible conversion functions from Quux[MyApp.r.E,MyApp.s.E] to Int
  (new Quux[r.E, s.E]): Int // fails due to pre-stripping implicits which
   ^
one error found
```

Which is clearly more informative (even if also a little baffling). I think
that ambiguity via multiple paths is sufficiently rare that the performance
benefits of this early pruning are likely to minimal and not enough to
outweigh improved error messages.

Because this change can result in a symbol _legitimately_ being revisited, via
a different prefix, we need to change the way that the a pruned path is
represented. Instead of using empty lists, as introduced in scala#5951 (which fixes
scala/bug#10081) we use explicit sentinels to mark
that a symbol has been visited at a given prefix yielding no additional
implicits. Testing this yielded scala/bug#10425
which appears to be a regression relative to 2.12.x. That's also fixed in this
PR.
@SethTisue SethTisue assigned milessabin and unassigned adriaanm Jan 23, 2018
@SethTisue SethTisue added this to the 2.13.0-M3 milestone Jan 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants