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

-Yretain-trees (and flags which imply it like -Ysafe-init) changes the types of trees returned by the reflection API #14195

Open
cchantep opened this issue Dec 31, 2021 · 7 comments
Labels

Comments

@cchantep
Copy link
Contributor

Compiler version

3.1.2-RC1-bin-20211222-c94b333-NIGHTLY

Minimized code

🤚 With -Ysafe-init in scalacOptions

import scala.deriving.Mirror
import scala.quoted.*
import scala.reflect.ClassTag

inline def test[A]: Unit = ${ testImpl[A] }

private def testImpl[A](
  using
    q: Quotes,
  t: Type[A]
): Expr[Unit] = {
  import q.reflect.*

  val tpr = TypeRepr.of[A]

  tpr.classSymbol.foreach {
    _.tree match {
      case cd: ClassDef =>
        val tpe = cd.constructor.returnTpt.tpe

        println(s"classTpe = ${tpe.show}")

      case _ =>
    }
  }

  '{ () }
}

// Execute test below:

final class Foo(val name: String)

test[Foo]

Output

classTpe = scala.Unit

Expectation

classTpe = Foo

⚠️ Expected result ok without -Ysafe-init

@liufengyun
Copy link
Contributor

The usage of the compiler flag -Yretain-trees has the same behavior as -Ysafe-init above.

Note that the usage of sym.tree is discouraged in meta-programming, as specified in the document here: https://scala-lang.org/api/3.x/scala/quoted/Quotes$reflectModule$SymbolMethods.html#tree-d26

The following code is a better way to achieve the goal without using sym.tree:

  tpr.classSymbol.foreach { sym =>
    tpr.memberType(sym.primaryConstructor) match {
      case MethodType(_, _, resTp) =>
        println(s"classTpe = ${resTp.show}")

      case _ =>
    }
  }

@cchantep
Copy link
Contributor Author

There are some (many) cases where you need to go through the tree, so the issue remains.

@smarter
Copy link
Member

smarter commented Jan 1, 2022

If you need to go through the tree this is likely due to something missing in the reflection API and should be reported as a bug, but regardless we should ensure that the behavior of tree.tpe does not change based on whether -Yretain-trees is enabled or not

@cchantep
Copy link
Contributor Author

cchantep commented Jan 1, 2022

If you need to go through the tree this is likely due to something missing in the reflection API and should be reported as a bug,

e.g. There is no way to find the known subtypes from a TypeRepr, whereas the .children is available on Symbol (so aTypeRepr.symbol.children), but then there is no clean way to resolve a TypeRepr for each of the children, except pattern matching of Symbol.tree.

but regardless we should ensure that the behavior of tree.tpe does not change based on whether -Yretain-trees is enabled or not

Yes

@smarter
Copy link
Member

smarter commented Jan 1, 2022

there is no clean way to resolve a TypeRepr for each of the children

I believe this is tracked by #13553

@smarter smarter changed the title Safe initialization prevent proper resolution of constructor Type in metaprogramming -Yretain-trees (and flags which imply it like -Ysafe-init) changes the types of trees returned by the reflection API Jan 1, 2022
@cchantep
Copy link
Contributor Author

cchantep commented Jan 1, 2022

there is no clean way to resolve a TypeRepr for each of the children

I believe this is tracked by #13553

No, #13553 is about resolving the TypeRepr for a Symbol, no resolving sub-types for a type using its TypeRepr (with something like TypeRepr.children).

@smarter
Copy link
Member

smarter commented Jan 1, 2022

I expect you would do tp.symbol.children.map(_.info) to get back the types.

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

4 participants