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

Fix back-quoted constructor params with identical prefixes #9008

Merged
merged 4 commits into from Oct 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 15 additions & 11 deletions src/compiler/scala/tools/nsc/transform/Constructors.scala
Expand Up @@ -474,18 +474,22 @@ abstract class Constructors extends Statics with Transform with TypingTransforme
def usesSpecializedField = intoConstructor.usesSpecializedField

// The constructor parameter corresponding to an accessor
def parameter(acc: Symbol): Symbol = parameterNamed(acc.unexpandedName.getterName)

// The constructor parameter with given name. This means the parameter
// has given name, or starts with given name, and continues with a `$` afterwards.
def parameterNamed(name: Name): Symbol = {
def matchesName(param: Symbol) = param.name == name || param.name.startsWith(s"${name}${nme.NAME_JOIN_STRING}")
def parameter(acc: Symbol): Symbol = {
import scala.util.Try
//works around the edge case where unexpandedName over-unexpands shenanigans like literal $$ or `$#`
def unexpanded = parameterNamed(acc.unexpandedName.getterName)
def expanded = parameterNamed(acc.getterName)
unexpanded.orElse(expanded).swap.map(abort).merge
}

primaryConstrParams filter matchesName match {
case Nil => abort(s"$name not in $primaryConstrParams")
case p :: _ => p
// The constructor parameter with given getter name. This means the parameter name
// decodes to the same name that the getter decodes to
def parameterNamed(name: Name): Either[String, Symbol] =
primaryConstrParams.filter(_.name.decodedName == name.decodedName) match {
case List(p) => Right(p)
case Nil => Left(s"No constructor parameter named $name (decoded to ${name.decodedName}) found in list of constructor parameters $primaryConstrParams (decoded to ${primaryConstrParams.map(_.decodedName)})")
case ps => Left(s"$name matches multiple constructor parameters $ps")
}
}

// A transformer for expressions that go into the constructor
object intoConstructor extends Transformer {
Expand Down Expand Up @@ -531,7 +535,7 @@ abstract class Constructors extends Statics with Transform with TypingTransforme
else if (canBeSupplanted(tree.symbol))
gen.mkAttributedIdent(parameter(tree.symbol)) setPos tree.pos
else if (tree.symbol.outerSource == clazz && !isDelayedInitSubclass)
gen.mkAttributedIdent(parameterNamed(nme.OUTER)) setPos tree.pos
gen.mkAttributedIdent(parameterNamed(nme.OUTER).fold(abort, identity)).setPos(tree.pos)
else
super.transform(tree)

Expand Down
Expand Up @@ -122,7 +122,7 @@ trait SyntheticMethods extends ast.TreeDSL {
)
}

def perElementMethod(name: Name, returnType: Type)(caseFn: Symbol => Tree): Tree =
def perElementMethod(name: Name, returnType: Type)(caseFn: Symbol => Tree): Tree =
createSwitchMethod(name, accessors.indices, returnType)(idx => caseFn(accessors(idx)))

def productElementNameMethod = {
Expand Down
3 changes: 3 additions & 0 deletions src/reflect/scala/reflect/internal/StdNames.scala
Expand Up @@ -437,6 +437,9 @@ trait StdNames {
* Look backward from the end of the string for "$$", and take the
* part of the string after that; but if the string is "$$$" or longer,
* be sure to retain the extra dollars.
* If the name happens to be a back quoted name containing literal $$
* or $ followed by an operator that gets encoded, go directly to compiler
* crash. Do not pass go and don't even think about collecting any $$
*/
def unexpandedName(name: Name): Name = name lastIndexOf "$$" match {
case 0 | -1 => name
Expand Down
16 changes: 12 additions & 4 deletions src/reflect/scala/reflect/internal/Symbols.scala
Expand Up @@ -2111,18 +2111,26 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
// handling of non-public parameters seems to change the order (see scala/bug#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)
// (need to undo name-mangling, including the sneaky trailing whitespace, and match longest first)
//
// 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 (_.name.dropLocal)
def nameStartsWithOrigDollar(name: Name, prefix: Name) =
name.startsWith(prefix) && name.length > prefix.length + 1 && name.charAt(prefix.length) == '$'
caseFieldAccessorsUnsorted.sortBy { acc =>
primaryNames indexWhere { orig =>
(acc.name == orig) || nameStartsWithOrigDollar(acc.name, orig)

def rec(remaningAccessors: List[Symbol], foundAccessors: List[(Symbol, Int)], remainingNames: List[(Name, Int)]): List[Symbol] = {
remaningAccessors match {
case Nil => foundAccessors.sortBy(_._2).map(_._1)
case acc :: tail => {
val i = remainingNames.collectFirst { case (name, i) if acc.name == name || nameStartsWithOrigDollar(acc.name, name) => i}
rec(tail, (acc, i.get) :: foundAccessors, remainingNames.filterNot { case (_, ii) => Some(ii) == i} )
}
}
}

rec(caseFieldAccessorsUnsorted.sortBy(s => -s.name.length), Nil, primaryNames.zipWithIndex.sortBy{ case (n, _) => -n.length})

}
private final def caseFieldAccessorsUnsorted: List[Symbol] = info.decls.toList.filter(_.isCaseAccessorMethod)

Expand Down
3 changes: 3 additions & 0 deletions test/files/run/t10625.check
@@ -0,0 +1,3 @@
1
1
Some(1)
8 changes: 8 additions & 0 deletions test/files/run/t10625.scala
@@ -0,0 +1,8 @@
case class WhyNot(`^$#`: Int)
object Test extends App {
val wn = WhyNot(1)
println(wn.`^$#`)
val WhyNot(i) = wn
println(i)
println(WhyNot.unapply(wn))
}
1 change: 1 addition & 0 deletions test/files/run/t8831.check
@@ -0,0 +1 @@
Right5(1,2,3,4,5)
47 changes: 47 additions & 0 deletions test/files/run/t8831.scala
@@ -0,0 +1,47 @@
case class Right(a: Int, `a b`: Int)
case class VeryRight(a: Int, `a b`: String)

case class Wrong(`a b`: Int, a: Int)
case class VeryWrong(`a b`: Int, a: String)
case class WrongDollar(a$: Int, a: Int)
case class VeryWrongDollar(a$: Int, a: String)
case class WrongQuotedDollar(`a$`: Int, a: Int)
case class WrongHyphenated(val `foo-bar`: Int, `foo`: Int)
case class VeryWrongHyphenated(val `foo-bar`: Int, `foo`: String)
case class WrongPlus(a_+ : Int, a_ : Int)
case class VeryWrongPlus(a_+ : Int, a_ : String)

case class Right5(b: Int, `a b`: Int, a: Int, `a `: Int, `a b c`: Int)

object Test {
def main(args: Array[String]): Unit = {
val r = Right(1, 2)
val w = Wrong(1, 2)
val wd = WrongDollar(1, 2)
val wh = WrongHyphenated(1, 2)
val wp = WrongPlus(1, 2)
assert(r.a == w.`a b`)
assert(r.a == wd.a$)
assert(r.a == wh.`foo-bar`)
assert(r.a == wp.a_+)
assert(r.`a b` == w.a)
assert(r.`a b` == wd.a)
assert(r.`a b` == wh.foo)
assert(r.`a b` == wp.a_)

val vr = VeryRight(1, "one")
val vw = VeryWrong(1, "one")
val vwd = VeryWrongDollar(1, "one")
val vwh = VeryWrongHyphenated(1, "one")
val vwp = VeryWrongPlus(1, "one")
assert(vr.a == vw.`a b`)
assert(vr.a == vwd.a$)
assert(vr.a == vwh.`foo-bar`)
assert(vr.a == vwp.a_+)
assert(vr.`a b` == vw.a)
assert(vr.`a b` == vwd.a)
assert(vr.`a b` == vwh.foo)
assert(vr.`a b` == vwp.a_)
println(Right5(1, 2, 3, 4, 5).toString())
}
}