Skip to content

Commit

Permalink
Unnamed and original apply survive rewrite
Browse files Browse the repository at this point in the history
  • Loading branch information
som-snytt committed Mar 6, 2024
1 parent f4df3a4 commit 195eeec
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 29 deletions.
40 changes: 21 additions & 19 deletions src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala
Original file line number Diff line number Diff line change
Expand Up @@ -55,20 +55,24 @@ trait NamesDefaults { self: Analyzer =>
qual: Option[Tree],
targs: List[Tree],
vargss: List[List[Tree]],
blockTyper: Typer
) { }
blockTyper: Typer,
original: Tree,
)
object NamedApplyBlock {
private[this] val tag = reflect.classTag[NamedApplyInfo]
def namedApplyInfo(t: Tree): Option[NamedApplyInfo] = t.attachments.get[NamedApplyInfo](tag)
def unapply(b: Tree): Option[NamedApplyInfo] = b match {
case _: Block => b.attachments.get[NamedApplyInfo](tag)
case _: Block => namedApplyInfo(b)
case _ => None
}
def apply(stats: List[Tree], expr: Tree)(nai: NamedApplyInfo): Block =
Block(stats, expr.updateAttachment(nai)).updateAttachment(nai)
}

private def nameOfNamedArg(arg: Tree) = Some(arg) collect { case NamedArg(Ident(name), _) => name }
def isNamedArg(arg: Tree) = arg match {
case NamedArg(Ident(_), _) => true
case _ => false
case _ => false
}

/** @param pos maps indices from old to new */
Expand All @@ -81,7 +85,7 @@ trait NamesDefaults { self: Analyzer =>
/** @param pos maps indices from new to old (!) */
private def reorderArgsInv[T: ClassTag](args: List[T], pos: Int => Int): List[T] = {
val argsArray = args.toArray
(argsArray.indices map (i => argsArray(pos(i)))).toList
argsArray.indices.map(i => argsArray(pos(i))).toList
}

/** returns `true` if every element is equal to its index */
Expand Down Expand Up @@ -203,17 +207,14 @@ trait NamesDefaults { self: Analyzer =>
else TypeApply(f, funTargs).setType(baseFun.tpe)
}

val b = Block(List(vd), baseFunTransformed)
.setType(baseFunTransformed.tpe).setPos(baseFun.pos.makeTransparent)
b.updateAttachment(NamedApplyInfo(Some(newQual), defaultTargs, Nil, blockTyper))
b
NamedApplyBlock(List(vd), baseFunTransformed)(NamedApplyInfo(Some(newQual), defaultTargs, Nil, blockTyper, tree))
.setType(baseFunTransformed.tpe)
.setPos(baseFun.pos.makeTransparent)
}

def blockWithoutQualifier(defaultQual: Option[Tree]) = {
val b = atPos(baseFun.pos)(Block(Nil, baseFun).setType(baseFun.tpe))
b.updateAttachment(NamedApplyInfo(defaultQual, defaultTargs, Nil, blockTyper))
b
}
def blockWithoutQualifier(defaultQual: Option[Tree]) =
atPos(baseFun.pos)(NamedApplyBlock(Nil, baseFun)(NamedApplyInfo(defaultQual, defaultTargs, Nil, blockTyper, tree)))
.setType(baseFun.tpe)

def moduleQual(pos: Position, classType: Type) = {
// prefix does 'normalize', which fixes #3384
Expand Down Expand Up @@ -345,7 +346,7 @@ trait NamesDefaults { self: Analyzer =>
val transformedFun = transformNamedApplication(typer, mode, pt)(fun, x => x)
if (transformedFun.isErroneous) setError(tree)
else {
val NamedApplyBlock(NamedApplyInfo(qual, targs, vargss, blockTyper)) = transformedFun: @unchecked
val NamedApplyBlock(NamedApplyInfo(qual, targs, vargss, blockTyper, _)) = transformedFun: @unchecked
val Block(stats, funOnly) = transformedFun: @unchecked

// type the application without names; put the arguments in definition-site order
Expand All @@ -370,19 +371,20 @@ trait NamesDefaults { self: Analyzer =>
val ref = gen.mkAttributedRef(vDef.symbol)
atPos(vDef.pos.focus) {
// for by-name parameters, the local value is a nullary function returning the argument
tpe.typeSymbol match {
val t = tpe.typeSymbol match {
case ByNameParamClass => Apply(ref, Nil)
case RepeatedParamClass => Typed(ref, Ident(tpnme.WILDCARD_STAR))
case _ => ref
}
t.setAttachments(origArg.attachments)
}
})
// cannot call blockTyper.typedBlock here, because the method expr might be partially applied only
val res = blockTyper.doTypedApply(tree, expr, refArgs, mode, pt)
res.setPos(res.pos.makeTransparent)
val block = Block(stats ::: valDefs.flatten, res).setType(res.tpe).setPos(tree.pos.makeTransparent)
block.updateAttachment(NamedApplyInfo(qual, targs, vargss :+ refArgs, blockTyper))
block
NamedApplyBlock(stats ::: valDefs.flatten, res)(NamedApplyInfo(qual, targs, vargss :+ refArgs, blockTyper, tree))
.setType(res.tpe)
.setPos(tree.pos.makeTransparent)
case _ => tree
}
}
Expand Down
17 changes: 14 additions & 3 deletions src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1703,14 +1703,14 @@ abstract class RefChecks extends Transform {

transform(qual)
case Apply(fn, args) =>
currentApplication = tree
// sensicality should be subsumed by the unreachability/exhaustivity/irrefutability
// analyses in the pattern matcher
if (!inPattern) {
checkImplicitViewOptionApply(tree.pos, fn, args)
checkSensible(tree.pos, fn, args) // TODO: this should move to preEraseApply, as reasoning about runtime semantics makes more sense in the JVM type system
checkNamedBooleanArgs(fn, args)
}
currentApplication = tree
tree
}

Expand All @@ -1733,6 +1733,17 @@ abstract class RefChecks extends Transform {
}
if (settings.warnNamedLiteral.value && !sym.isJavaDefined && !args.isEmpty) {
def isUnnamedArg(t: Tree) = t.hasAttachment[UnnamedArg.type]
def isDefaultArg(t: Tree) = t.symbol != null && (
t.symbol.isDefaultGetter || t.symbol.name.startsWith(nme.NAMEDARG_PREFIX) && {
analyzer.NamedApplyBlock.namedApplyInfo(currentApplication) match {
case Some(analyzer.NamedApplyInfo(_, _, _, _, original)) =>
val treeInfo.Applied(_, _, argss) = original
val orig = argss.head(t.symbol.name.decoded.stripPrefix(nme.NAMEDARG_PREFIX).toInt - 1)
orig.symbol != null && orig.symbol.isDefaultGetter
case _ => false
}
}
)
val params = sym.paramLists(applyDepth)
val numBools = params.count(_.tpe == BooleanTpe)
def onlyLeadingBool = numBools == 1 && params.head.tpe == BooleanTpe
Expand All @@ -1746,7 +1757,7 @@ abstract class RefChecks extends Transform {
case Literal(Constant(_: Boolean)) =>
if (isUnnamedArg(arg)) (ap :: acc._1, acc._2 + 1) else acc
case _ =>
if (arg.symbol != null && arg.symbol.isDefaultGetter) (acc._1, acc._2 + 1) else acc
if (isDefaultArg(arg)) (acc._1, acc._2 + 1) else acc
}
case _ => acc
}
Expand All @@ -1756,7 +1767,7 @@ abstract class RefChecks extends Transform {
unnamed.reverse.foreach {
case (arg, param) =>
val msg = s"Boolean literals should be passed using named argument syntax for parameter ${param.name}."
val action = runReporting.codeAction("name boolean literal", arg.pos.withEnd(arg.pos.start), s"${param.name} = ", msg)
val action = runReporting.codeAction("name boolean literal", arg.pos.focusStart, s"${param.name} = ", msg)
runReporting.warning(arg.pos, msg, WarningCategory.WFlagNamedLiteral, sym, action)
case _ =>
}
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/typechecker/Typers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3819,7 +3819,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
val fun1 = transformNamedApplication(Typer.this, mode, pt)(fun, x => x)
if (fun1.isErroneous) duplErrTree
else {
val NamedApplyBlock(NamedApplyInfo(qual, targs, previousArgss, _)) = fun1: @unchecked
val NamedApplyBlock(NamedApplyInfo(qual, targs, previousArgss, _, _)) = fun1: @unchecked
val blockIsEmpty = fun1 match {
case Block(Nil, _) =>
// if the block does not have any ValDef we can remove it. Note that the call to
Expand Down
13 changes: 8 additions & 5 deletions test/files/neg/named-booleans-relaxed.check
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,21 @@ named-booleans-relaxed.scala:72: warning: Boolean literals should be passed usin
named-booleans-relaxed.scala:72: warning: Boolean literals should be passed using named argument syntax for parameter down. [quickfixable]
def g4 = f(42, false, true) // warn, swappable
^
named-booleans-relaxed.scala:78: warning: Boolean literals should be passed using named argument syntax for parameter up. [quickfixable]
named-booleans-relaxed.scala:79: warning: Boolean literals should be passed using named argument syntax for parameter up. [quickfixable]
def rev3 = rev(42, reverse=true, false) // warn, unnamed could mean either param with default
^
named-booleans-relaxed.scala:79: warning: Boolean literals should be passed using named argument syntax for parameter reverse. [quickfixable]
named-booleans-relaxed.scala:80: warning: Boolean literals should be passed using named argument syntax for parameter reverse. [quickfixable]
def rev4 = rev(42, false, true, false) // warn, swappable
^
named-booleans-relaxed.scala:79: warning: Boolean literals should be passed using named argument syntax for parameter up. [quickfixable]
named-booleans-relaxed.scala:80: warning: Boolean literals should be passed using named argument syntax for parameter up. [quickfixable]
def rev4 = rev(42, false, true, false) // warn, swappable
^
named-booleans-relaxed.scala:79: warning: Boolean literals should be passed using named argument syntax for parameter down. [quickfixable]
named-booleans-relaxed.scala:80: warning: Boolean literals should be passed using named argument syntax for parameter down. [quickfixable]
def rev4 = rev(42, false, true, false) // warn, swappable
^
named-booleans-relaxed.scala:81: warning: Boolean literals should be passed using named argument syntax for parameter reverse. [quickfixable]
def rev5 = rev(42, true, down=true) // warn, out of order so it's a named block, otherwise same as rev3
^
error: No warnings can be incurred under -Werror.
13 warnings
14 warnings
1 error
4 changes: 3 additions & 1 deletion test/files/neg/named-booleans-relaxed.scala
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,12 @@ class Defaulting {
def g3 = f(42, false) // warn, unnamed could mean either param with default
def g4 = f(42, false, true) // warn, swappable

def rev(n: Int, reverse: Boolean = false, up: Boolean = true, down: Boolean = false) = if (down) n+1 else if (up) n-1 else n
def rev(n: Int, reverse: Boolean = false, up: Boolean = true, down: Boolean = false) =
if (!reverse) f(n, up, down) else if (down) n+1 else if (up) n-1 else n
def rev0 = rev(42) // nowarn, all defaults
def rev1 = rev(42, up=false) // nowarn, named or defaults
def rev2 = rev(42, true, up=false, down=true) // nowarn, in param order so not a named block, unnamed is last remaining param
def rev3 = rev(42, reverse=true, false) // warn, unnamed could mean either param with default
def rev4 = rev(42, false, true, false) // warn, swappable
def rev5 = rev(42, true, down=true) // warn, out of order so it's a named block, otherwise same as rev3
}

0 comments on commit 195eeec

Please sign in to comment.