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

runtime/compile time reflection doesn't see all knownDirectSubclasses #7046

Closed
scabug opened this Issue Jan 30, 2013 · 21 comments

Comments

Projects
None yet
2 participants
@scabug
Copy link

scabug commented Jan 30, 2013

Unlike, say, annotations or flags, knownDirectSubclasses doesn't get auto-populated and requires a symbol to be pre-initialized to work correctly.

import scala.reflect.runtime.universe._
import scala.reflect.runtime.{currentMirror => cm}

sealed class C
class D extends C
class E extends C

object Test extends App {
  val c = cm.staticClass("C")
  println(c.knownDirectSubclasses)
  c.typeSignature
  println(c.knownDirectSubclasses)
}
19:43 ~/Projects/Kepler_7046/sandbox (ticket/7046)$ scalac Test.scala && scala Test
Set()
Set(class D, class E)
@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jan 30, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7046?orig=1
Reporter: @xeno-by
Affected Versions: 2.10.0, 2.11.0
Other Milestones: 2.12.1

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jan 31, 2013

@xeno-by said:
Probably should a very easy fix. Will try to get it once we're discussing synchronization.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jan 31, 2013

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Feb 4, 2013

@xeno-by said:
Fixed in scala/scala@2403d1d

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Apr 11, 2013

@adriaanm said:
Actually, this only works when you query knownDirectSubclasses after those trees have been typed, so macros that occur before the class C in your example has been type checked won't see its subclasses.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 20, 2013

@JamesIry said:
2.10.2 is about to be cut. Kicking down the road and un-assigning to foster work stealing.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jun 19, 2013

@retronym said:
Test case for compile time reflection in #7588

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Dec 10, 2013

Valentin Churavy (wallnuss) said (edited on Dec 10, 2013 6:46:40 AM UTC):
I still run into this on 2.10.3 and the work around with calling c.typeSignature before c.knownDirectSubclasses doesn't work for the reason Adriaan noticed. Forcing a recompile on only the parts which use the macro solve this error, but that is of course not really satisfactory.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Dec 10, 2013

@adriaanm said:
Sorry, this is not something we can fix due the way the typechecker is implemented. The "fix" would be to remove this method from the reflection API.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Dec 10, 2013

Valentin Churavy (wallnuss) said:
Would it be possible to reschedule the classes where the macro is used for compilation after the TypeCheck has occurred?

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Dec 10, 2013

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Apr 18, 2014

@xeno-by said:
Actually, I think we can fix this for an important subset of macros: https://groups.google.com/forum/#!topic/scala-user/oJP9aqs88rM

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Aug 3, 2014

@xeno-by said:
See another test case at #8776

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jun 6, 2016

@milessabin said:
@adriaanm can you elaborate on "this is not something we can fix due the way the typechecker is implemented" ... are there any specific obstacles you have in mind?

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jul 11, 2016

@Blaisorblade said:

  1. Since this is a design issue, do scala.meta and/or Dotty change things here? I wonder if we need a label like redesign for such issues.
  2. @milessabin the design issue is mentioned in the discussion links, in particular in https://groups.google.com/d/msg/scala-internals/LRfKffsPxVA/db_jmsa7FO4J:

knownDirectSubclasses relies on a full top-down pass having been completed before you call it.

Technically, we could provide a onTypecheckCompilationUnit that you could use, but this isn't currently planned for the macro api (well, I'll let Eugene comment on that, but I'm not aware of any plans here -- in any case, 2.11 is in ramp down)

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jul 12, 2016

@xeno-by said:
Scala.meta relies on the underlying platform typechecker, so it won't change anything here unless the implementation of scalac/dotc changes.

One way to fix this issue without changing the typechecker is to move macro expansion to after typer instead of during typer. We'll have to experiment with this, because there are some technical difficulties with implementing that (e.g. implicit inference not working well after typer, etc).

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jul 15, 2016

@milessabin said:
WIP PR here: scala/scala#5284

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Aug 11, 2016

@cchantep said:
Could the following be used as workaround in some case (seems to work in my case as far as I can see).

val cls: ClassSymbol = sub.asClass

println(s"knownDirectSubclasses = ${cls.knownDirectSubclasses}")
// print "knownDirectSubclasses = Set()"

val subsub = cls.owner.typeSignature.decls.filter {
  case c: ClassSymbol =>
    cls != c && c.selfType.baseClasses.contains(cls)

  case _ => false
}

println(s"subsub = $subsub")
// print the actual sub classes
@scabug

This comment has been minimized.

Copy link
Author

scabug commented Aug 26, 2016

@milessabin said:
Fix backported to 2.11.8 is available here.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Oct 18, 2016

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jan 9, 2017

@adriaanm said:
To track future refinements, please open a new issue. Closing this one to mark our progress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.