Skip to content

Commit 3136e53

Browse files
committed
Merge branch 'pull-1352-reformatted' into 2.10.x
# By Martin Odersky * pull-1352-reformatted: Disabled failing build manager tests. New test case for SI-6337 New test case for closing SI-6385 Value classes: eliminated half-boxing Cleanup of OverridingPairs Fixes SI-6260
2 parents d834d90 + d87592d commit 3136e53

24 files changed

+258
-57
lines changed

src/compiler/scala/tools/nsc/Global.scala

+2
Original file line numberDiff line numberDiff line change
@@ -1094,6 +1094,7 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
10941094

10951095
// TODO - trim these to the absolute minimum.
10961096
@inline final def afterErasure[T](op: => T): T = afterPhase(currentRun.erasurePhase)(op)
1097+
@inline final def afterPostErasure[T](op: => T): T = afterPhase(currentRun.posterasurePhase)(op)
10971098
@inline final def afterExplicitOuter[T](op: => T): T = afterPhase(currentRun.explicitouterPhase)(op)
10981099
@inline final def afterFlatten[T](op: => T): T = afterPhase(currentRun.flattenPhase)(op)
10991100
@inline final def afterIcode[T](op: => T): T = afterPhase(currentRun.icodePhase)(op)
@@ -1403,6 +1404,7 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
14031404
val specializePhase = phaseNamed("specialize")
14041405
val explicitouterPhase = phaseNamed("explicitouter")
14051406
val erasurePhase = phaseNamed("erasure")
1407+
val posterasurePhase = phaseNamed("posterasure")
14061408
// val lazyvalsPhase = phaseNamed("lazyvals")
14071409
val lambdaliftPhase = phaseNamed("lambdalift")
14081410
// val constructorsPhase = phaseNamed("constructors")

src/compiler/scala/tools/nsc/transform/Erasure.scala

+76-22
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ abstract class Erasure extends AddInterfaces
167167
case tp => tp :: Nil
168168
}
169169

170+
private def isErasedValueType(tpe: Type) = tpe.isInstanceOf[ErasedValueType]
171+
170172
/** The Java signature of type 'info', for symbol sym. The symbol is used to give the right return
171173
* type for constructors.
172174
*/
@@ -373,18 +375,18 @@ abstract class Erasure extends AddInterfaces
373375
}
374376
}
375377

376-
class ComputeBridges(owner: Symbol) {
378+
class ComputeBridges(unit: CompilationUnit, root: Symbol) {
377379
assert(phase == currentRun.erasurePhase, phase)
378380

379381
var toBeRemoved = immutable.Set[Symbol]()
380-
val site = owner.thisType
382+
val site = root.thisType
381383
val bridgesScope = newScope
382384
val bridgeTarget = mutable.HashMap[Symbol, Symbol]()
383385
var bridges = List[Tree]()
384386

385387
val opc = beforeExplicitOuter {
386-
new overridingPairs.Cursor(owner) {
387-
override def parents = List(owner.info.firstParent)
388+
new overridingPairs.Cursor(root) {
389+
override def parents = List(root.info.firstParent)
388390
override def exclude(sym: Symbol) = !sym.isMethod || sym.isPrivate || super.exclude(sym)
389391
}
390392
}
@@ -402,8 +404,58 @@ abstract class Erasure extends AddInterfaces
402404
(bridges, toBeRemoved)
403405
}
404406

407+
/** Check that a bridge only overrides members that are also overridden by the original member.
408+
* This test is necessary only for members that have a value class in their type.
409+
* Such members are special because their types after erasure and after post-erasure differ/.
410+
* This means we generate them after erasure, but the post-erasure transform might introduce
411+
* a name clash. The present method guards against these name clashes.
412+
*
413+
* @param member The original member
414+
* @param other The overidden symbol for which the bridge was generated
415+
* @param bridge The bridge
416+
*/
417+
def checkBridgeOverrides(member: Symbol, other: Symbol, bridge: Symbol): Boolean = {
418+
def fulldef(sym: Symbol) =
419+
if (sym == NoSymbol) sym.toString
420+
else s"$sym: ${sym.tpe} in ${sym.owner}"
421+
var noclash = true
422+
def clashError(what: String) = {
423+
noclash = false
424+
unit.error(
425+
if (member.owner == root) member.pos else root.pos,
426+
s"""bridge generated for member ${fulldef(member)}
427+
|which overrides ${fulldef(other)}
428+
|clashes with definition of $what;
429+
|both have erased type ${afterPostErasure(bridge.tpe)}""".stripMargin)
430+
}
431+
for (bc <- root.baseClasses) {
432+
if (settings.debug.value)
433+
afterPostErasure(println(
434+
s"""check bridge overrides in $bc
435+
${bc.info.nonPrivateDecl(bridge.name)}
436+
${site.memberType(bridge)}
437+
${site.memberType(bc.info.nonPrivateDecl(bridge.name) orElse IntClass)}
438+
${(bridge.matchingSymbol(bc, site))}""".stripMargin))
439+
440+
def overriddenBy(sym: Symbol) =
441+
sym.matchingSymbol(bc, site).alternatives filter (sym => !sym.isBridge)
442+
for (overBridge <- afterPostErasure(overriddenBy(bridge))) {
443+
if (overBridge == member) {
444+
clashError("the member itself")
445+
} else {
446+
val overMembers = overriddenBy(member)
447+
if (!overMembers.exists(overMember =>
448+
afterPostErasure(overMember.tpe =:= overBridge.tpe))) {
449+
clashError(fulldef(overBridge))
450+
}
451+
}
452+
}
453+
}
454+
noclash
455+
}
456+
405457
def checkPair(member: Symbol, other: Symbol) {
406-
val otpe = erasure(owner)(other.tpe)
458+
val otpe = erasure(root)(other.tpe)
407459
val bridgeNeeded = afterErasure (
408460
!(other.tpe =:= member.tpe) &&
409461
!(deconstMap(other.tpe) =:= deconstMap(member.tpe)) &&
@@ -417,24 +469,29 @@ abstract class Erasure extends AddInterfaces
417469
return
418470

419471
val newFlags = (member.flags | BRIDGE) & ~(ACCESSOR | DEFERRED | LAZY | lateDEFERRED)
420-
val bridge = other.cloneSymbolImpl(owner, newFlags) setPos owner.pos
472+
val bridge = other.cloneSymbolImpl(root, newFlags) setPos root.pos
421473

422474
debuglog("generating bridge from %s (%s): %s to %s: %s".format(
423475
other, flagsToString(newFlags),
424476
otpe + other.locationString, member,
425-
erasure(owner)(member.tpe) + member.locationString)
477+
erasure(root)(member.tpe) + member.locationString)
426478
)
427479

428480
// the parameter symbols need to have the new owner
429481
bridge setInfo (otpe cloneInfo bridge)
430482
bridgeTarget(bridge) = member
431-
afterErasure(owner.info.decls enter bridge)
432-
if (other.owner == owner) {
433-
afterErasure(owner.info.decls.unlink(other))
434-
toBeRemoved += other
483+
484+
if (!(member.tpe exists (_.typeSymbol.isDerivedValueClass)) ||
485+
checkBridgeOverrides(member, other, bridge)) {
486+
afterErasure(root.info.decls enter bridge)
487+
if (other.owner == root) {
488+
afterErasure(root.info.decls.unlink(other))
489+
toBeRemoved += other
490+
}
491+
492+
bridgesScope enter bridge
493+
bridges ::= makeBridgeDefDef(bridge, member, other)
435494
}
436-
bridgesScope enter bridge
437-
bridges ::= makeBridgeDefDef(bridge, member, other)
438495
}
439496

440497
def makeBridgeDefDef(bridge: Symbol, member: Symbol, other: Symbol) = afterErasure {
@@ -466,7 +523,7 @@ abstract class Erasure extends AddInterfaces
466523
val rhs = member.tpe match {
467524
case MethodType(Nil, ConstantType(c)) => Literal(c)
468525
case _ =>
469-
val sel: Tree = Select(This(owner), member)
526+
val sel: Tree = Select(This(root), member)
470527
val bridgingCall = (sel /: bridge.paramss)((fun, vparams) => Apply(fun, vparams map Ident))
471528

472529
maybeWrap(bridgingCall)
@@ -480,8 +537,6 @@ abstract class Erasure extends AddInterfaces
480537

481538
private def isPrimitiveValueType(tpe: Type) = isPrimitiveValueClass(tpe.typeSymbol)
482539

483-
private def isErasedValueType(tpe: Type) = tpe.isInstanceOf[ErasedValueType]
484-
485540
private def isDifferentErasedValueType(tpe: Type, other: Type) =
486541
isErasedValueType(tpe) && (tpe ne other)
487542

@@ -814,7 +869,6 @@ abstract class Erasure extends AddInterfaces
814869
* but their erased types are the same.
815870
*/
816871
private def checkNoDoubleDefs(root: Symbol) {
817-
def afterErasure[T](op: => T): T = atPhase(phase.next.next)(op)
818872
def doubleDefError(sym1: Symbol, sym2: Symbol) {
819873
// the .toString must also be computed at the earlier phase
820874
val tpe1 = afterRefchecks(root.thisType.memberType(sym1))
@@ -830,7 +884,7 @@ abstract class Erasure extends AddInterfaces
830884
sym2 + ":" + afterRefchecks(tpe2.toString) +
831885
(if (sym2.owner == root) " at line " + (sym2.pos).line else sym2.locationString) +
832886
"\nhave same type" +
833-
(if (afterRefchecks(tpe1 =:= tpe2)) "" else " after erasure: " + afterErasure(sym1.tpe)))
887+
(if (afterRefchecks(tpe1 =:= tpe2)) "" else " after erasure: " + afterPostErasure(sym1.tpe)))
834888
sym1.setInfo(ErrorType)
835889
}
836890

@@ -840,7 +894,7 @@ abstract class Erasure extends AddInterfaces
840894
if (e.sym.isTerm) {
841895
var e1 = decls.lookupNextEntry(e)
842896
while (e1 ne null) {
843-
if (afterErasure(e1.sym.info =:= e.sym.info)) doubleDefError(e.sym, e1.sym)
897+
if (afterPostErasure(e1.sym.info =:= e.sym.info)) doubleDefError(e.sym, e1.sym)
844898
e1 = decls.lookupNextEntry(e1)
845899
}
846900
}
@@ -854,7 +908,7 @@ abstract class Erasure extends AddInterfaces
854908
|| !sym.hasTypeAt(currentRun.refchecksPhase.id))
855909

856910
override def matches(sym1: Symbol, sym2: Symbol): Boolean =
857-
afterErasure(sym1.tpe =:= sym2.tpe)
911+
afterPostErasure(sym1.tpe =:= sym2.tpe)
858912
}
859913
while (opc.hasNext) {
860914
if (!afterRefchecks(
@@ -902,7 +956,7 @@ abstract class Erasure extends AddInterfaces
902956
private def bridgeDefs(owner: Symbol): (List[Tree], immutable.Set[Symbol]) = {
903957
assert(phase == currentRun.erasurePhase, phase)
904958
debuglog("computing bridges for " + owner)
905-
new ComputeBridges(owner) compute()
959+
new ComputeBridges(unit, owner) compute()
906960
}
907961

908962
def addBridges(stats: List[Tree], base: Symbol): List[Tree] =
@@ -1000,7 +1054,7 @@ abstract class Erasure extends AddInterfaces
10001054
preEraseIsInstanceOf
10011055
} else if (fn.symbol.owner.isRefinementClass && !fn.symbol.isOverridingSymbol) {
10021056
ApplyDynamic(qualifier, args) setSymbol fn.symbol setPos tree.pos
1003-
} else if (fn.symbol.isMethodWithExtension) {
1057+
} else if (fn.symbol.isMethodWithExtension && !fn.symbol.tpe.isErroneous) {
10041058
Apply(gen.mkAttributedRef(extensionMethods.extensionMethod(fn.symbol)), qualifier :: args)
10051059
} else {
10061060
tree

src/compiler/scala/tools/nsc/transform/ExtensionMethods.scala

+2-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ abstract class ExtensionMethods extends Transform with TypingTransformers {
7070
val companionInfo = imeth.owner.companionModule.info
7171
val candidates = extensionNames(imeth) map (companionInfo.decl(_))
7272
val matching = candidates filter (alt => normalize(alt.tpe, imeth.owner) matches imeth.tpe)
73-
assert(matching.nonEmpty, "no extension method found for "+imeth+" among "+candidates+"/"+extensionNames(imeth))
73+
assert(matching.nonEmpty,
74+
s"no extension method found for $imeth:${imeth.tpe}+among ${candidates map (c => c.name+":"+c.tpe)} / ${extensionNames(imeth)}")
7475
matching.head
7576
}
7677

src/compiler/scala/tools/nsc/transform/OverridingPairs.scala

+22-17
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,11 @@ abstract class OverridingPairs {
104104

105105
/** A map from baseclasses of <base> to ints, with smaller ints meaning lower in
106106
* linearization order.
107+
* symbols that are not baseclasses map to -1.
107108
*/
108-
private val index = new mutable.HashMap[Symbol, Int]
109+
private val index = new mutable.HashMap[Symbol, Int] {
110+
override def default(key: Symbol) = -1
111+
}
109112

110113
// Note: overridingPairs can be called at odd instances by the Eclipse plugin
111114
// Soemtimes symbols are not yet defined and we get missing keys.
@@ -133,28 +136,30 @@ abstract class OverridingPairs {
133136
{ for (i <- List.range(0, size))
134137
subParents(i) = new BitSet(size);
135138
for (p <- parents) {
136-
index get p.typeSymbol match {
137-
case Some(pIndex) =>
138-
for (bc <- p.baseClasses)
139-
if (p.baseType(bc) =:= self.baseType(bc))
140-
index get bc match {
141-
case Some(bcIndex) =>
142-
include(subParents(bcIndex), pIndex)
143-
case None =>
144-
}
145-
else debuglog("SKIPPING "+p+" -> "+p.baseType(bc)+" / "+self.baseType(bc)+" from "+base)
146-
case None =>
147-
}
139+
val pIndex = index(p.typeSymbol)
140+
if (pIndex >= 0)
141+
for (bc <- p.baseClasses)
142+
if (p.baseType(bc) =:= self.baseType(bc)) {
143+
val bcIndex = index(bc)
144+
if (bcIndex >= 0)
145+
include(subParents(bcIndex), pIndex)
146+
}
148147
}
149148
}
150149

151150
/** Do `sym1` and `sym2` have a common subclass in `parents`?
152151
* In that case we do not follow their overriding pairs
153152
*/
154-
private def hasCommonParentAsSubclass(sym1: Symbol, sym2: Symbol) = (
155-
for (index1 <- index get sym1.owner ; index2 <- index get sym2.owner) yield
156-
intersectionContainsElementLeq(subParents(index1), subParents(index2), index1 min index2)
157-
).exists(_ == true)
153+
private def hasCommonParentAsSubclass(sym1: Symbol, sym2: Symbol) = {
154+
val index1 = index(sym1.owner)
155+
(index1 >= 0) && {
156+
val index2 = index(sym2.owner)
157+
(index2 >= 0) && {
158+
intersectionContainsElementLeq(
159+
subParents(index1), subParents(index2), index1 min index2)
160+
}
161+
}
162+
}
158163

159164
/** The scope entries that have already been visited as overridden
160165
* (maybe excluded because of hasCommonParentAsSubclass).

src/reflect/scala/reflect/internal/transform/Erasure.scala

+10-12
Original file line numberDiff line numberDiff line change
@@ -203,28 +203,26 @@ trait Erasure {
203203
def specialErasure(sym: Symbol)(tp: Type): Type =
204204
if (sym != NoSymbol && sym.enclClass.isJavaDefined)
205205
erasure(sym)(tp)
206-
else if (sym.isTerm && sym.owner.isDerivedValueClass)
207-
specialErasureAvoiding(sym.owner, tp)
208-
else if (sym.isValue && sym.owner.isMethodWithExtension)
209-
specialErasureAvoiding(sym.owner.owner, tp)
206+
else if (sym.isClassConstructor)
207+
specialConstructorErasure(sym.owner, tp)
210208
else
211209
specialScalaErasure(tp)
212210

213-
def specialErasureAvoiding(clazz: Symbol, tpe: Type): Type = {
211+
def specialConstructorErasure(clazz: Symbol, tpe: Type): Type = {
214212
tpe match {
215213
case PolyType(tparams, restpe) =>
216-
specialErasureAvoiding(clazz, restpe)
214+
specialConstructorErasure(clazz, restpe)
217215
case ExistentialType(tparams, restpe) =>
218-
specialErasureAvoiding(clazz, restpe)
216+
specialConstructorErasure(clazz, restpe)
219217
case mt @ MethodType(params, restpe) =>
220218
MethodType(
221-
cloneSymbolsAndModify(params, specialErasureAvoiding(clazz, _)),
222-
if (restpe.typeSymbol == UnitClass) erasedTypeRef(UnitClass)
223-
else specialErasureAvoiding(clazz, (mt.resultType(mt.paramTypes))))
219+
cloneSymbolsAndModify(params, specialScalaErasure),
220+
specialConstructorErasure(clazz, restpe))
224221
case TypeRef(pre, `clazz`, args) =>
225222
typeRef(pre, clazz, List())
226-
case _ =>
227-
specialScalaErasure(tpe)
223+
case tp =>
224+
assert(clazz == ArrayClass || tp.isError, s"unexpected constructor erasure $tp for $clazz")
225+
specialScalaErasure(tp)
228226
}
229227
}
230228

test/files/buildmanager/overloaded_1/A.scala renamed to test/files/disabled/A.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ trait As {
33
override def foo = this /// Shouldn't cause the change
44
override def foo(act: List[D]) = this
55
}
6-
6+
77
abstract class D{
88
def foo: D = this
99
def foo(act: List[D]) = this

test/files/neg/t6260.check

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
t6260.scala:3: error: bridge generated for member method apply: (x$1: Box[X])Box[Y] in anonymous class $anonfun
2+
which overrides method apply: (v1: T1)R in trait Function1
3+
clashes with definition of the member itself;
4+
both have erased type (v1: Object)Object
5+
((bx: Box[X]) => new Box(f(bx.x)))(this)
6+
^
7+
t6260.scala:8: error: bridge generated for member method apply: (x$1: Box[X])Box[Y] in anonymous class $anonfun
8+
which overrides method apply: (v1: T1)R in trait Function1
9+
clashes with definition of the member itself;
10+
both have erased type (v1: Object)Object
11+
((bx: Box[X]) => new Box(f(bx.x)))(self)
12+
^
13+
two errors found

test/files/neg/t6260.scala

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
class Box[X](val x: X) extends AnyVal {
2+
def map[Y](f: X => Y): Box[Y] =
3+
((bx: Box[X]) => new Box(f(bx.x)))(this)
4+
}
5+
6+
object Test {
7+
def map2[X, Y](self: Box[X], f: X => Y): Box[Y] =
8+
((bx: Box[X]) => new Box(f(bx.x)))(self)
9+
10+
def main(args: Array[String]) {
11+
val f = (x: Int) => x + 1
12+
val g = (x: String) => x + x
13+
14+
map2(new Box(42), f)
15+
new Box("abc") map g
16+
}
17+
}

test/files/neg/t6385.check

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
t6385.scala:12: error: bridge generated for member method x: ()C[T] in class C
2+
which overrides method x: ()C[T] in trait AA
3+
clashes with definition of the member itself;
4+
both have erased type ()Object
5+
def x = this
6+
^
7+
one error found

test/files/neg/t6385.scala

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
object N {
2+
def main(args: Array[String]) {
3+
val y: AA[Int] = C(2)
4+
val c: Int = y.x.y
5+
println(c)
6+
}
7+
}
8+
trait AA[T] extends Any {
9+
def x: C[T]
10+
}
11+
case class C[T](val y: T) extends AnyVal with AA[T] {
12+
def x = this
13+
}
+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
valueclasses-pavlov.scala:8: error: double definition:
2+
method foo:(x: Box2)String and
3+
method foo:(x: String)String at line 7
4+
have same type after erasure: (x: String)String
5+
def foo(x: Box2) = "foo(Box2): ok"
6+
^
7+
one error found

0 commit comments

Comments
 (0)