Fix #2964: Refresh names for anonymous classes #2999
Conversation
nicolasstucki
force-pushed
the
fix-#2964
branch
from
ba22b2b to
1c61bdf
Compare
|
|
||
| override def phaseName = "liftedClasses" | ||
|
|
||
| override def runsAfterGroupsOf: Set[Class[_ <: Phases.Phase]] = Set(classOf[Flatten]) |
There was a problem hiding this comment.
It's not really flatten, but RestoreScopes.
| @@ -16,6 +17,8 @@ class TransformWildcards extends MiniPhaseTransform with IdentityDenotTransforme | |||
|
|
|||
| override def phaseName = "transformWildcards" | |||
|
|
|||
| override def runsAfter: Set[Class[_ <: Phases.Phase]] = Set(classOf[LiftedClasses]) | |||
There was a problem hiding this comment.
Why is there a dependency between those two phases?
There was a problem hiding this comment.
Only checked experimentally. It failed if I put it afterward, therefore I added the explicit runsAfter to make this evident to anyone that tries to reorder phases.
| else tree | ||
|
|
||
| private def needsRefresh(sym: Symbol)(implicit ctx: Context): Boolean = | ||
| sym.isClass && !sym.is(Package) && sym.name.is(UniqueName) |
There was a problem hiding this comment.
Do we have packages with UniqueName?
There was a problem hiding this comment.
Why do you apply it only to classes, don't we have the same issue with methods that are lifted out?
There was a problem hiding this comment.
class Foo {
def ann = {
def foo() = 1;
}
}
class Bar {
def ann = {
def foo() = 1;
}
}public class Foo {
public void ann() {
}
private static int foo$1() {
return 1;
}
}
public class Bar {
public void ann() {
}
private static int foo$2() { // <--- same issue
return 1;
}
}| /** Renames lifted classes to local numbering scheme */ | ||
| class LiftedClasses extends MiniPhaseTransform with SymTransformer { thisTransformer => | ||
|
|
||
| override def phaseName = "liftedClasses" |
|
@DarkDimius the requested changes have been made. |
nicolasstucki
dismissed
DarkDimius’s stale review
Code has been rewritten since last review
|
|
||
| /** Refreshes the number of the name based on the full name of the symbol */ | ||
| private def refreshedName(sym: Symbol)(implicit ctx: Context): Name = { | ||
| lazy val rewriteUnique: PartialFunction[Name, Name] = { |
There was a problem hiding this comment.
Oh I see, that's a funny way to write recursion 😄
There was a problem hiding this comment.
Do you know a cleaner way to define recursive partial functions?
There was a problem hiding this comment.
I would do it with a def
There was a problem hiding this comment.
Changed it. Though now it will create a new instance of the function each iteration, I guess this should not happen too often.
nicolasstucki
force-pushed
the
fix-#2964
branch
from
37e667e to
fe7ac0a
Compare
No description provided.