Skip to content

Type.of drops an opaque type info if macro called from the same level type type defined #18283

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

Closed
chuwy opened this issue Jul 24, 2023 · 9 comments · Fixed by #18287
Closed
Assignees
Labels
area:metaprogramming:quotes Issues related to quotes and splices

Comments

@chuwy
Copy link

chuwy commented Jul 24, 2023

Compiler version

3.3.0, 3.3.1-RC4

Minimized code

// Macro.scala
package test

import scala.quoted.*

object Macro:
  transparent inline def getType[T] =
    ${ getTypeImpl[T] }
  private def getTypeImpl[T: Type](using Quotes): Expr[Any] =
    import quotes.reflect.*
  
    val tpe = TypeRepr.of[T]
    val reprShow = tpe.show
    tpe.asType match
      case '[t] =>
        val typeShow = TypeRepr.of[t].show
        Expr((reprShow, typeShow))


// Aux.scala
package test

opaque type Id = Long

object Task:
  opaque type Title = String

def run =
  println((Macro.getType[Id], Macro.getType[Task.Title]))

// console
test.run

Output

((test.Aux$package.Id,scala.Long),(test.Task.Title,test.Task.Title))

Expectation

((test.Aux$package.Id,test.Aux$package.Id),(test.Task.Title,test.Task.Title))

I do expect to have the opaque type preserved in all situations.

I still haven't managed to catch the exact conditions when it fails (not sure that "same level" is to blame). Here's another situation where the behavior doesn't match the expectation:

package test

opaque type Id = Long

object Task:
  opaque type Title = String

  def run =
    println((Macro.getType[Id], Macro.getType[Title]))

Output:

((test.Aux$package.Id,test.Aux$package.Id),(test.Task.Title,java.lang.String))

And with this run it has expected result:

println((Macro.getType[Id], Macro.getType[Task.Title]))   // Task.Title is better qualified
@chuwy chuwy added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Jul 24, 2023
@chuwy chuwy changed the title Type.of drops an opaque type info if macro called from an inner object Type.of drops an opaque type info if macro called from the same level type type defined Jul 24, 2023
@nicolasstucki nicolasstucki added area:metaprogramming:quotes Issues related to quotes and splices and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Jul 25, 2023
@nicolasstucki
Copy link
Contributor

nicolasstucki commented Jul 25, 2023

The opaque type specification states

opaque type T >: L <: U = R

Inside the scope of the alias definition, the alias is transparent: T is treated as a normal alias of R.

This implies that in reference to an opaque type at the same level the is type defined, is seen as a type alias. Therefore it is fine if it is dealiased.

Note that the dealiased only happens on the quoted pattern matching. In that case t is the a precise type that results on all the constraints found in the pattern itself. Which is clearly the dealiased type.

Furthermore, in both cases Type.of[T] and TypeRepr.of[T] return the reference to the opaque type. This implies that there is a way to see if the code referred to the opaque type (not guaranteed because of dealiassing).

nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Jul 25, 2023
@chuwy
Copy link
Author

chuwy commented Jul 26, 2023

I think that would be an acceptable if there was no "not guaranteed" part at least. Because right now the outcome is just non-deterministic.

Macro.getType[Aux.Id]

and

Macro.getType[Id]

Provide different results (given that Id and run are defined at the same places and difference is just how we refer to the Id type) and just checked there's no reference to opaque type when I pass there Id.

Sorry if that sounds over-dramatic, but it's a huge letdown from opaque types - as result I cannot use them in my case classes at all, given that at any point the classes could be used in macros and compile-time error will be super hard (impossible for users) to debug.

@nicolasstucki
Copy link
Contributor

What is the intended use case?

@chuwy
Copy link
Author

chuwy commented Jul 26, 2023

Example with skunk-tables - if I define in table class some member id: Id (with it being opaque type Id = UUID) and then try to do something with the member:

val x: Id = ???
table.get(_.id == id)

The compiler complains that it expected id to be UUID, but it's in fact Id, so I was trying to use opaque types here as newtypes, but converting it back somewhere defeats the purpose. There are many places where I managed to fix it, e.g. I replaced in quotidian:

  def tupleToList: List[quotes.reflect.TypeRepr] =
    import quotes.reflect.*
    self.asType match
      case '[t *: ts]    => TypeRepr.of[t] :: TypeRepr.of[ts].tupleToList
      case '[EmptyTuple] => Nil

To

def tupleToList: List[quotes.reflect.TypeRepr] =
  import quotes.reflect.*
  self match
    case AppliedType(TypeRef(ThisType(_),"*:"), List(head, tail)) if tail.show == "scala.Tuple$package.EmptyTuple" =>
      head :: Nil
    case AppliedType(TypeRef(ThisType(_),"*:"), List(head, tail @ AppliedType(_, _))) =>
      head :: tail.tupleToList
    case end if end.show == "scala.Tuple$package.EmptyTuple" =>
      Nil

But there are other places where I don't know how to get rid of Type.of and asType

@nicolasstucki
Copy link
Contributor

The compiler complains that it expected id to be UUID

Could you show a small self-contained example of where this goes wrong?

@bishabosha
Copy link
Member

Sorry if that sounds over-dramatic, but it's a huge letdown from opaque types - as result I cannot use them in my case classes at all, given that at any point the classes could be used in macros and compile-time error will be super hard (impossible for users) to debug.

is it not really possible to define that opaque type somewhere else, (e.g. a nested object) and then re-export it from the scope where it was originally?

@chuwy
Copy link
Author

chuwy commented Jul 26, 2023

is it not really possible to define that opaque type somewhere else

This is what I did in the end, yes:

opaque type TaskId = UUID
object Task:
  type Id = TaskId
final case class Task(id: Task.Id)

It works, but my broader concern that other users (of skunk-tables, or generally any macro-heavy lib that reflects on product types and summons implicit for their members) will have hard times debugging that.

And even broader concern about opaque types in general. This is not the first time I hit a similar problem (#17211) and I'm wondering if my expectations of opaque types are wrong or nobody has raised this.

Could you show a small self-contained example of where this goes wrong?

Do you mean without mentioned lib?

@nicolasstucki
Copy link
Contributor

If you want to see the underlying type of TaskId anywhere, should you define it as type TaskId and just assume it is a UUID?

bishabosha pushed a commit that referenced this issue Jul 27, 2023
@bishabosha
Copy link
Member

This would be more safe:

type TaskId = Task.Id
object Task:
  opaque type Id = UUID
final case class Task(id: Task.Id)

Kordyjan pushed a commit that referenced this issue Dec 1, 2023
Kordyjan pushed a commit that referenced this issue Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metaprogramming:quotes Issues related to quotes and splices
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants