Skip to content
Browse files

SI-7035 Centralize case field accessor sorting.

It is both burdensome and dangerous to expect callers
to reorder these. This was seen in the field permutation
in the unapply method; a regression in 2.10.0.
  • Loading branch information...
1 parent 02963d7 commit 9afae59f0e46bed0acf40d043d3ee2a0e0ef432f @retronym retronym committed Jan 28, 2013
View
20 src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala
@@ -746,26 +746,8 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
// reference the (i-1)th case accessor if it exists, otherwise the (i-1)th tuple component
override protected def tupleSel(binder: Symbol)(i: Int): Tree = { import CODE._
- // caseFieldAccessors is messed up after typers (reversed, names mangled for non-public fields)
- // TODO: figure out why...
val accessors = binder.caseFieldAccessors
- // luckily, the constrParamAccessors are still sorted properly, so sort the field-accessors using them
- // (need to undo name-mangling, including the sneaky trailing whitespace)
- val constrParamAccessors = binder.constrParamAccessors
-
- def indexInCPA(acc: Symbol) =
- constrParamAccessors indexWhere { orig =>
- // patmatDebug("compare: "+ (orig, acc, orig.name, acc.name, (acc.name == orig.name), (acc.name startsWith (orig.name append "$"))))
- val origName = orig.name.toString.trim
- val accName = acc.name.toString.trim
- (accName == origName) || (accName startsWith (origName + "$"))
- }
-
- // patmatDebug("caseFieldAccessors: "+ (accessors, binder.caseFieldAccessors map indexInCPA))
- // patmatDebug("constrParamAccessors: "+ constrParamAccessors)
-
- val accessorsSorted = accessors sortBy indexInCPA
- if (accessorsSorted isDefinedAt (i-1)) REF(binder) DOT accessorsSorted(i-1)
+ if (accessors isDefinedAt (i-1)) REF(binder) DOT accessors(i-1)
else codegen.tupleSel(binder)(i) // this won't type check for case classes, as they do not inherit ProductN
}
View
9 src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala
@@ -78,14 +78,7 @@ trait SyntheticMethods extends ast.TreeDSL {
else templ
}
- val originalAccessors = clazz.caseFieldAccessors
- // private ones will have been renamed -- make sure they are entered
- // in the original order.
- def accessors = clazz.caseFieldAccessors sortBy { acc =>
- originalAccessors indexWhere { orig =>
- (acc.name == orig.name) || (acc.name startsWith (orig.name append "$"))
- }
- }
+ def accessors = clazz.caseFieldAccessors
val arity = accessors.size
// If this is ProductN[T1, T2, ...], accessorLub is the lub of T1, T2, ..., .
// !!! Hidden behind -Xexperimental due to bummer type inference bugs.
View
23 src/reflect/scala/reflect/internal/Symbols.scala
@@ -1728,8 +1728,27 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
/** For a case class, the symbols of the accessor methods, one for each
* argument in the first parameter list of the primary constructor.
* The empty list for all other classes.
- */
- final def caseFieldAccessors: List[Symbol] =
+ *
+ * This list will be sorted to correspond to the declaration order
+ * in the constructor parameter
+ */
+ final def caseFieldAccessors: List[Symbol] = {
+ // We can't rely on the ordering of the case field accessors within decls --
+ // handling of non-public parameters seems to change the order (see SI-7035.)
+ //
+ // Luckily, the constrParamAccessors are still sorted properly, so sort the field-accessors using them
+ // (need to undo name-mangling, including the sneaky trailing whitespace)
+ //
+ // The slightly more principled approach of using the paramss of the
+ // primary constructor leads to cycles in, for example, pos/t5084.scala.
+ val primaryNames = constrParamAccessors.map(acc => nme.dropLocalSuffix(acc.name))
+ caseFieldAccessorsUnsorted.sortBy { acc =>
+ primaryNames indexWhere { orig =>
+ (acc.name == orig) || (acc.name startsWith (orig append "$"))
+ }
+ }
+ }
+ private final def caseFieldAccessorsUnsorted: List[Symbol] =
(info.decls filter (_.isCaseAccessorMethod)).toList
final def constrParamAccessors: List[Symbol] =
View
15 test/files/pos/t7035.scala
@@ -0,0 +1,15 @@
+case class Y(final var x: Int, final private var y: String, final val z1: Boolean, final private val z2: Any) {
+
+ import Test.{y => someY}
+ List(someY.x: Int, someY.y: String, someY.z1: Boolean, someY.z2: Any)
+ someY.y = ""
+}
+
+object Test {
+ val y = Y(0, "", true, new {})
+ val unapp: Option[(Int, String, Boolean, Any)] = // was (Int, Boolean, String, Any) !!
+ Y.unapply(y)
+
+ val Y(a, b, c, d) = y
+ List(a: Int, b: String, c: Boolean, d: Any)
+}

0 comments on commit 9afae59

Please sign in to comment.
Something went wrong with that request. Please try again.