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

Regression for implicit search in mixed Java/Scala compilation in scala-tsi/scala-tsi #21303

Closed
WojciechMazur opened this issue Jul 30, 2024 · 8 comments
Assignees
Labels
area:implicits related to implicits area:typer itype:bug regression This worked in a previous version but doesn't anymore

Comments

@WojciechMazur
Copy link
Contributor

WojciechMazur commented Jul 30, 2024

Compiler version

Last good release: 3.6.0-RC1-bin-20240719-af933c4-NIGHTLY
First bad release: 3.6.0-RC1-bin-20240723-46ff151-NIGHTLY
Bisect points to 2facda0

Minimized code

// Test.scala

import scala.deriving.Mirror
import scala.compiletime.*
import scala.reflect.ClassTag
import scala.annotation.implicitNotFound


trait TSType[T]
object TSType extends DefaultTSTypes with TSTypeMacros

trait TSNamedType[T] extends TSType[T]

trait DefaultTSTypes extends JavaTSTypes
trait JavaTSTypes {
  implicit def javaEnumTSType[E <: java.lang.Enum[E]: ClassTag]: TSNamedType[E] = ??? 
}
object DefaultTSTypes extends DefaultTSTypes
trait TSTypeMacros {
  inline given [T: Mirror.Of]: TSType[T] = derived[T]
  inline def derived[T](using m: Mirror.Of[T]): TSType[T] = {
    val elemInstances = summonAll[m.MirroredElemTypes]
    ???
  }

  private inline def summonAll[T <: Tuple]: List[TSType[_]] = {
    inline erasedValue[T] match {
      case _: EmptyTuple => Nil
      case _: (t *: ts)  => summonInline[TSType[t]] :: summonAll[ts]
    }
  }
}

@main def Test = summon[TSType[JavaEnum]]
// JavaEnum.java
public enum JavaEnum { ABC, DEF, GHI }

Output

[error] -- [E172] Type Error: /Users/wmazur/projects/community-build3/repo/src/main/scala/com/scalatsi/TSType.scala:34:41 
[error] 34 |@main def Test = summon[TSType[JavaEnum]]
[error]    |                                         ^
[error]    |No given instance of type TSType[(JavaEnum.ABC : JavaEnum)] was found.
[error]    |I found:
[error]    |
[error]    |    TSType.given_TSType_T[(JavaEnum.ABC : JavaEnum)](
[error]    |      JavaEnum.$asInstanceOf[
[error]    |        
[error]    |          scala.deriving.Mirror.Sum{
[error]    |            type MirroredMonoType = (JavaEnum.ABC : JavaEnum);
[error]    |              type MirroredType = (JavaEnum.ABC : JavaEnum);
[error]    |              type MirroredLabel = ("JavaEnum" : String);
[error]    |              type MirroredElemTypes = ((JavaEnum.ABC : JavaEnum),
[error]    |                (JavaEnum.DEF : JavaEnum), (JavaEnum.GHI : JavaEnum));
[error]    |              type MirroredElemLabels = (("ABC" : String), ("DEF" : String),
[error]    |                ("GHI" : String))
[error]    |          }
[error]    |        
[error]    |      ]
[error]    |    )
[error]    |
[error]    |But given instance given_TSType_T in trait TSTypeMacros does not match type TSType[(JavaEnum.ABC : JavaEnum)].
[error]    |----------------------------------------------------------------------------
[error]    |Inline stack trace
[error]    |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]    |This location contains code that was inlined from TSType.scala:29
[error] 29 |      case _: (t *: ts)  => summonInline[TSType[t]] :: summonAll[ts]
[error]    |                            ^^^^^^^^^^^^^^^^^^^^^^^
[error]    |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]    |This location contains code that was inlined from TSType.scala:29
[error] 22 |    val elemInstances = summonAll[m.MirroredElemTypes]
[error]    |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]    |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]    |This location contains code that was inlined from TSType.scala:29
[error] 20 |  inline given [T: Mirror.Of]: TSType[T] = derived[T]
[error]    |                                           ^^^^^^^^^^
[error]     ----------------------------------------------------------------------------

When replacing

  inline given [T: Mirror.Of]: TSType[T] = derived[T]

to

  inline given [T: Mirror.Of]: TSType[T] = ???

it fails with

rror] -- [E119] Type Error: /Users/wmazur/projects/community-build3/repo/src/test/scala/com/scalatsi/EnumTests.scala:7:38 
[error] 7 |  val x = implicitly[TSType[JavaEnum]]
[error]   |                                      ^
[error]   |                                Java defined class JavaEnum is not a value

Expectation

Should choose dedicated javaEnumTSType implicit and compile

@WojciechMazur WojciechMazur added itype:bug area:typer area:implicits related to implicits regression This worked in a previous version but doesn't anymore labels Jul 30, 2024
@WojciechMazur
Copy link
Contributor Author

In #21273 for revision f236ce9 (revert from main) / 1ea5485 (last commit from PR used in OpenCB) it fails with

Error:  -- [E172] Type Error: /build/repo/src/test/scala/com/scalatsi/EnumTests.scala:24:50 
Error:  24 |      val generated = implicitly[TSType[JavaEnum]]
Error:     |                                                  ^
Error:     |Ambiguous given instances: both method javaEnumTSType in trait JavaTSTypes and given instance given_TSType_T in trait TSTypeMacros match type com.scalatsi.TSType[com.scalatsi.types.JavaEnum] of parameter e of method implicitly in object Predef
Error:  one error found

@Gedochao
Copy link
Contributor

cc @EugeneFlesselle @odersky

@EugeneFlesselle
Copy link
Contributor

Minimization:

// Test.scala

trait TSType[T]

object TSType:
  implicit def javaEnumTSType[E <: java.lang.Enum[E]]: TSType[E] = ???

  inline given derived[T]: TSType[T] = compiletime.error("used given derived")

@main def Test = 
  summon[TSType[JavaEnum]] // error starting from 2facda0

This is as expected by the intent to prioritize givens over implicits from #19300

@EugeneFlesselle
Copy link
Contributor

The issues will be directly affected by the changes we decide to go with in #21273.
It may or may not be that this is still affected by the changes after it, the project from which this was minimized can be used as part of the empirical data determining if the changes break to much code. We can decide whether or not to clause this issue once we have chosen an approach.

@odersky
Copy link
Contributor

odersky commented Jul 31, 2024

Yes this is ambiguous under the new rules even with #21273 or its replacement #21305. I added test cases to #21305. The problem is fixed by declaring the JavaEnum given as

implicit def javaEnumTSType[E <: java.lang.Enum[E]: ClassTag]: TSType[E] = ???

instead of

implicit def javaEnumTSType[E <: java.lang.Enum[E]: ClassTag]: TSNamedType[E] = ???

odersky added a commit to dotty-staging/dotty that referenced this issue Jul 31, 2024
@odersky
Copy link
Contributor

odersky commented Jul 31, 2024

Another thing that works is to duplicate the JavaEnum implicit

  given javaEnumTSType[E <: java.lang.Enum[E]: ClassTag]: TSType[E] = ???
  given javaEnumTSNamedType[E <: java.lang.Enum[E]: ClassTag]: TSNamedType[E] = ???

That way, we can still get a TSNamedType from a JavaEnum is we explicitly request one:

  summon[TSNamedType[JavaEnum]]

@odersky odersky self-assigned this Aug 1, 2024
@odersky
Copy link
Contributor

odersky commented Aug 1, 2024

I am assigning to myself so that I can follow this and close when we decide that a mitigation is impossible.

@odersky
Copy link
Contributor

odersky commented Aug 21, 2024

I am closing the issue because we already have tests (added in #21339) that verify the behavior discussed in the comments behavior. If the behavior changes the tests will fail and we can update the info on the issue.

@odersky odersky closed this as not planned Won't fix, can't repro, duplicate, stale Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:implicits related to implicits area:typer itype:bug regression This worked in a previous version but doesn't anymore
Projects
None yet
Development

No branches or pull requests

4 participants