-
Notifications
You must be signed in to change notification settings - Fork 19
Drop needless source-recompile in Power #50
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
Conversation
It's unnecessary to recompile sources to gain symbol override information, simply (IIUC) starting the presentation compiler on the resulting classpath will recover that information from parsing the scala signatures/pickles in the classfile bytecode. Many thanks to Olaf for the advice.
lrytz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 👍
| val gSymbols = g.inverseSemanticdbSymbols(sSym.value) | ||
| val gSym = gSymbols.find(gSym => g.semanticdbSymbol(gSym) == sSym.value).getOrElse(g.NoSymbol) | ||
| if (gSym.info.exists(g.definitions.NothingTpe == _)) | ||
| gSym.overrides.lastOption.getOrElse(gSym) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know what this special case covers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It comes from scalacenter/scalafix@49cf3ca, but I'm failing to grok: it talks about "when the override method has Nothing in the inferred type", hasn't inference finished by now? Is it Nothing because the code is carefully avoid the type to be forced, or because the rest of the type inference happens later in a later step of the type-checker?
Either way, let me know if it would be good to try to drop this or change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It traverses all components of the symbol's type, and if anything refers to Nothing, it takes the root overridden method. I have no idea why, maybe to get rid of some ??? overrides?
I'm fine either way, was just wondering if you knew more.
scala> class A { override def toString = ??? }
scala> val gSym = typeOf[A].member(TermName("toString"))
val gSym: $r.intp.global.Symbol = method toString
scala> gSym.owner
val res9: $r.intp.global.Symbol = class A
scala> val r = if (gSym.info.exists(definitions.NothingTpe == _)) gSym.overrides.lastOption.getOrElse(gSym) else gSym
val r: $r.intp.global.Symbol = method toString
scala> r.owner
val res10: $r.intp.global.Symbol = class Any
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, the motivation for this workaround was to avoid inserting Nothing into the source code. In the case of ExplicitResultTypes, there is no good answer on whether it's best to insert the type signature of the submethod or the supermethod. Both approaches have their pros and cons.
I don't think this workaround is needed in this PR here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see.
It's unnecessary to recompile sources to gain symbol override
information, simply (IIUC) starting the presentation compiler on the
resulting classpath will recover that information from parsing the scala
signatures/pickles in the classfile bytecode.
Many thanks to Olaf for the advice.