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

Confusing suggested import involving inline implicit #9685

Open
mpilquist opened this issue Aug 31, 2020 · 9 comments · Fixed by #15025 or #17924
Open

Confusing suggested import involving inline implicit #9685

mpilquist opened this issue Aug 31, 2020 · 9 comments · Fixed by #15025 or #17924
Labels
area:reporting Error reporting including formatting, implicit suggestions, etc itype:bug
Milestone

Comments

@mpilquist
Copy link
Contributor

Minimized code

import munit.FunSuite

class Example extends FunSuite {
  1.asdf
}

Output

[error] 4 |  1.asdf
[error]   |  ^^^^^^
[error]   |value asdf is not a member of Int, but could be made available as an extension method.
[error]   |
[error]   |The following import might make progress towards fixing the problem:
[error]   |
[error]   |  import munit.Clue.generate
[error]   |
[error] one error found

Expectation

Expected:

[error] 2 |  1.asdf
[error]   |  ^^^^^^
[error]   |  value asdf is not a member of Int

Dotty 0.27.0-RC1 suggests munit.Clue.generate to fix this issue. The clue method and Clue class are defined as:

inline implicit def generate[T](value: T): Clue[T] = ${ clueImpl('value) }

class Clue[+T](
    val source: String,
    val value: T,
    val valueType: String
)
@sjrd
Copy link
Member

sjrd commented Aug 31, 2020

I've seen this problem before with another inline implicit def. I think the suggestion mechanism believes that inline defs can return anything, this potentially a more precise result that happens to be helpful in the given context. Whereas in fact blackbox inline defs cannot return a more specific type.

@smarter smarter changed the title Confusing suggested import Confusing suggested import involving inline implicit Aug 31, 2020
@smarter smarter added the area:reporting Error reporting including formatting, implicit suggestions, etc label Aug 31, 2020
@nicolasstucki
Copy link
Contributor

I could not reproduce it, I only get the error. @mpilquist could you write a self-contained code snipped that reproduces the issue?

@mpilquist
Copy link
Contributor Author

@nicolasstucki I'll work on a standalone reproduction but here's an executable one in the meantime: https://scastie.scala-lang.org/bG9JkRnARomdC2pCPD5eAQ

@mpilquist
Copy link
Contributor Author

mpilquist commented Sep 1, 2020

OK this reproduces:

Clue.scala:

package foo

import scala.language.implicitConversions

class Clue[+T](val value: T)

object Clue {
  import scala.quoted._

  inline implicit def generate[T](value: T): Clue[T] = ${ clueImpl('value) }

  def clueImpl[T:Type](value: Expr[T])(using qctx: QuoteContext): Expr[Clue[T]] = '{ new Clue($value) }
}

Main.scala:

package foo

object Main {
  // If this def is removed, there's no suggestion to import Clue.generate
  def toClue[A](a: A): Clue[A] = Clue.generate(a)

  1.asdf
}

nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Apr 25, 2022
@nicolasstucki
Copy link
Contributor

@olafurpg the original issue is already fixed. Could you check your use case with the current master to see if that one also got fixed? Otherwise, we should open another issue with that new scenario.

@tgodzik
Copy link
Contributor

tgodzik commented Feb 2, 2023

Looks like we have a new case that's failing:
https://github.com/alessandrocandolini/munit-clue-confusing-suggestion

@tgodzik tgodzik reopened this Feb 2, 2023
julienrf added a commit to scalacenter/dotty that referenced this issue Jun 6, 2023
julienrf added a commit to scalacenter/dotty that referenced this issue Jun 13, 2023
julienrf added a commit to scalacenter/dotty that referenced this issue Jun 16, 2023
nicolasstucki added a commit that referenced this issue Jun 19, 2023
…ss they are transparent (#17924)

In the first commit, I add a failing test:

~~~ scala
import scala.language.implicitConversions

class Foo

object Foo:
  inline implicit def toFoo(x: Int): Foo = Foo()

object Usage:
  1.asdf // error
~~~

We expect that code to not compile but the error reported by the
compiler is confusing as it suggests importing `Foo.toFoo` to resolve
the compilation error. You can see this in the [test
report](https://github.com/lampepfl/dotty/actions/runs/5254687053/jobs/9493612604#step:9:1859).

The problem comes from the fact that currently when the compiler checks
whether an implicit conversion is applicable to an expression that fails
to compile, it does not take into account the expected result type
(here, `? { def asdf: ? }`) if the candidate is an `inline` definition.

Instead, I believe the expected result type should be taken into account
unless the candidate is a `transparent inline`. I make this change in
the second commit, which makes the test pass.

Fixes #9685
@julienrf
Copy link
Contributor

Can the fix be backported to 3.3.x? Who should I ask for?

julienrf added a commit to scalacenter/dotty that referenced this issue Jul 6, 2023
We apply the full fix under `-source future` only, and a partial fix otherwise.

This is a followup of scala#17924 that fixes the source incompatibilities reported in scala#18130.

We test that the behavior under `-source future` still fixes scala#9685, and that without `-source future` we fix both scala#9685 and scala#18130.
julienrf added a commit to scalacenter/dotty that referenced this issue Jul 6, 2023
We apply the full fix under `-source future` only, and a partial fix otherwise.

This is a followup of scala#17924 that fixes the source incompatibilities reported in scala#18123.

We test that the behavior under `-source future` still fixes scala#9685, and that without `-source future` we fix both scala#9685 and scala#18123.
julienrf added a commit to scalacenter/dotty that referenced this issue Jul 6, 2023
We apply the full fix under `-source future` only, and a partial fix otherwise.

This is a followup of scala#17924 that fixes the source incompatibilities reported in scala#18123.

We test that the behavior under `-source future` still fixes scala#9685, and that without `-source future` we fix both scala#9685 and scala#18123.
julienrf added a commit to scalacenter/dotty that referenced this issue Jul 6, 2023
We apply the full fix under `-source future` only, and a partial fix otherwise.

This is a followup of scala#17924 that fixes the source incompatibilities reported in scala#18123.

We test that the behavior under `-source future` still fixes scala#9685, and that without `-source future` we fix both scala#9685 and scala#18123.
@Kordyjan Kordyjan added this to the 3.4.0 milestone Aug 1, 2023
nicolasstucki added a commit that referenced this issue Mar 12, 2024
Proposed in
#19415 (comment)

Fixes #19415

Reverts #18130 and
#17924.

 #18130 is still closed as it was introduced by #17924.

This reopens #9685
@nicolasstucki
Copy link
Contributor

We had to revert the commit that closed this issue to fix #19415

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:reporting Error reporting including formatting, implicit suggestions, etc itype:bug
Projects
None yet
8 participants