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

Ambient opaque type breaks MkFocus #1306

Open
armanbilge opened this issue Oct 11, 2022 · 4 comments
Open

Ambient opaque type breaks MkFocus #1306

armanbilge opened this issue Oct 11, 2022 · 4 comments
Labels
Milestone

Comments

@armanbilge
Copy link
Contributor

armanbilge commented Oct 11, 2022

//> using scala "3.2.0"
//> using lib "dev.optics::monocle-macro::3.1.0"

trait Qux

object Foo {

  opaque type Bar = Qux

  case class Baz(id: String)
  object Baz {
    implicit val bazId: monocle.Lens[Foo.Baz, String] = new monocle.Focus.MkFocus[Foo.Baz].apply(_.id)
  }

}
[error] ./bug.scala:12:57: Found:    monocle.PIso[?1.CAP, ?2.CAP, String, String]
[error] Required: monocle.Lens[Foo.Baz, String]
[error] 
[error] where:    ?1 is an unknown value of type scala.runtime.TypeBox[Nothing, Foo.type{Bar = Qux}#Baz]
[error]           ?2 is an unknown value of type scala.runtime.TypeBox[Nothing, Foo.type{Bar = Qux}#Baz]
[error]     implicit val bazId: monocle.Lens[Foo.Baz, String] = new monocle.Focus.MkFocus[Foo.Baz].apply(_.id)
[error]                                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@armanbilge armanbilge changed the title Ambient opaque type breaks GenLens Ambient opaque type breaks MkFocus Oct 11, 2022
armanbilge added a commit to armanbilge/Monocle that referenced this issue Oct 11, 2022
@armanbilge
Copy link
Contributor Author

Workaround.

//> using scala "3.2.0"
//> using lib "dev.optics::monocle-macro::3.1.0"

trait Qux

object Foo {

  opaque type Bar = Qux

  case class Baz(id: String)
  object Baz {
    implicit val bazId: monocle.Lens[Foo.Baz, String] = {
      type Bla = Foo.Baz // <-- workaround
      new monocle.Focus.MkFocus[Bla].apply(_.id)
    }
  }

}

@julien-truffaut
Copy link
Member

Users aren't expected to use MkFocus directly. Do you have the same issue with Focus?

Focus[Foo.Baz, String](_.id)

It is really weird that a useless opaque type break the macro. Was it also the case in previous version of Scala 3?

Thanks for reporting the issue.

@armanbilge
Copy link
Contributor Author

armanbilge commented Oct 13, 2022

Users aren't expected to use MkFocus directly. Do you have the same issue with Focus?

Right, sorry :) if you check the edit history the initial report was in terms of GenLens. Switching to MkFocus was intended as minimization.

It is really weird that a useless opaque type break the macro. Was it also the case in previous version of Scala 3?

I think so, but I will doube-check. Edit: I can reproduce with Scala 3.1.0.

FWIW this is likely a compiler bug.

@yilinwei
Copy link
Collaborator

yilinwei commented Dec 22, 2023

I think this is possibly intended behaviour by the compiler but weird as heck.

Here's the error message which we get in 3.3:

[error] 10 |    implicit val bazId: monocle.Lens[Foo.Baz, String] = monocle.Focus[Foo.Baz](_.id)
[error]    |                                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]    |               Found:    monocle.PIso[? <: Foo.type{type Bar = Qux}#Baz,
[error]    |                 ? <: Foo.type{type Bar = Qux}#Baz, String, String]
[error]    |               Required: monocle.Lens[Foo.Baz, String]

It looks as though we capture the opaque type within the Foo type (widen and dealias don't do anything btw) and since PIso doesn't have variance, we get issues with the explicit type ascription. I think that Foo{ ... }#Baz is a subtype of Foo.Baz but I'm not entirely sure about subtyping rules in this case honestly and need to double check.

The real fix is to add variance properly if it works imo, but I think we ran into issues with that previously with inference problems in scala 2. @julien-truffaut will remember better than me.

We could probably strip the captured opaque type for now.

@yilinwei yilinwei added this to the 3.3 milestone Jan 3, 2024
@yilinwei yilinwei added the bug label Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants