Skip to content

Change order of evaluation for default parameters #11609

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

Merged
merged 4 commits into from
Mar 5, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/ast/tpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -817,8 +817,8 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
val ownerAcc = new TreeAccumulator[immutable.Set[Symbol]] {
def apply(ss: immutable.Set[Symbol], tree: Tree)(using Context) = tree match {
case tree: DefTree =>
if (tree.symbol.exists) ss + tree.symbol.owner
else ss
val sym = tree.symbol
if sym.exists && !sym.owner.is(Package) then ss + sym.owner else ss
case _ =>
foldOver(ss, tree)
}
Expand Down
14 changes: 12 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -750,13 +750,23 @@ trait Applications extends Compatibility {
}
private def sameSeq[T <: Trees.Tree[?]](xs: List[T], ys: List[T]): Boolean = firstDiff(xs, ys) < 0

/** An argument is safe if it is a pure expression or a default getter call
* If all arguments are safe, no reordering is necessary
*/
def isSafeArg(arg: Tree) =
isPureExpr(arg)
|| arg.isInstanceOf[RefTree | Apply | TypeApply] && arg.symbol.name.is(DefaultGetterName)

val result: Tree = {
var typedArgs = typedArgBuf.toList
def app0 = cpy.Apply(app)(normalizedFun, typedArgs) // needs to be a `def` because typedArgs can change later
val app1 =
if (!success) app0.withType(UnspecifiedErrorType)
else {
if (!sameSeq(args, orderedArgs.dropWhile(_ eq EmptyTree)) && !isJavaAnnotConstr(methRef.symbol)) {
if !sameSeq(args, orderedArgs)
&& !isJavaAnnotConstr(methRef.symbol)
&& !typedArgs.forall(isSafeArg)
then
// need to lift arguments to maintain evaluation order in the
// presence of argument reorderings.

Expand Down Expand Up @@ -787,7 +797,7 @@ trait Applications extends Compatibility {
argDefBuf.zip(impureArgIndices), (arg, idx) => originalIndex(idx)).map(_._1)
}
liftedDefs ++= orderedArgDefs
}
end if
if (sameSeq(typedArgs, args)) // trick to cut down on tree copying
typedArgs = args.asInstanceOf[List[Tree]]
assignType(app0, normalizedFun, typedArgs)
Expand Down
8 changes: 4 additions & 4 deletions compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -459,11 +459,11 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler
val denot = qualifier.tpe.member(name.toTermName)
assert(!denot.isOverloaded, s"The symbol `$name` is overloaded. The method Select.unique can only be used for non-overloaded symbols.")
withDefaultPos(tpd.Select(qualifier, name.toTermName))
def overloaded(qualifier: Term, name: String, targs: List[TypeRepr], args: List[Term]): Apply =
withDefaultPos(tpd.applyOverloaded(qualifier, name.toTermName, args, targs, Types.WildcardType).asInstanceOf[Apply])
def overloaded(qualifier: Term, name: String, targs: List[TypeRepr], args: List[Term]): Term =
withDefaultPos(tpd.applyOverloaded(qualifier, name.toTermName, args, targs, Types.WildcardType))

def overloaded(qualifier: Term, name: String, targs: List[TypeRepr], args: List[Term], returnType: TypeRepr): Apply =
withDefaultPos(tpd.applyOverloaded(qualifier, name.toTermName, args, targs, returnType).asInstanceOf[Apply])
def overloaded(qualifier: Term, name: String, targs: List[TypeRepr], args: List[Term], returnType: TypeRepr): Term =
withDefaultPos(tpd.applyOverloaded(qualifier, name.toTermName, args, targs, returnType))
def copy(original: Tree)(qualifier: Term, name: String): Select =
tpd.cpy.Select(original)(qualifier, name.toTermName)
def unapply(x: Select): (Term, String) =
Expand Down
3 changes: 3 additions & 0 deletions compiler/test/dotc/pos-test-pickling.blacklist
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,6 @@ i9793.scala

# lazy_implicit symbol has different position after pickling
i8182.scala

# local lifted value in annotation argument has different position after pickling
i2797a
4 changes: 2 additions & 2 deletions library/src/scala/quoted/Quotes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -776,10 +776,10 @@ trait Quotes { self: runtime.QuoteUnpickler & runtime.QuoteMatching =>
def unique(qualifier: Term, name: String): Select

/** Call an overloaded method with the given type and term parameters */
def overloaded(qualifier: Term, name: String, targs: List[TypeRepr], args: List[Term]): Apply
def overloaded(qualifier: Term, name: String, targs: List[TypeRepr], args: List[Term]): Term

/** Call an overloaded method with the given type and term parameters */
def overloaded(qualifier: Term, name: String, targs: List[TypeRepr], args: List[Term], returnType: TypeRepr): Apply
def overloaded(qualifier: Term, name: String, targs: List[TypeRepr], args: List[Term], returnType: TypeRepr): Term

def copy(original: Tree)(qualifier: Term, name: String): Select

Expand Down
2 changes: 2 additions & 0 deletions tests/run/i11571.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
17
42
8 changes: 8 additions & 0 deletions tests/run/i11571.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import util.chaining.scalaUtilChainingOps
object Test extends App {
def x = 42.tap(println(_))
def y = 27.tap(println(_))
def z = 17.tap(println(_))
def f(i: Int = x, j: Int = y) = i + j
f(j = z)
}
6 changes: 3 additions & 3 deletions tests/semanticdb/expect/NamedApplyBlock.expect.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ package example
object NamedApplyBlockMethods/*<-example::NamedApplyBlockMethods.*/ {
val local/*<-example::NamedApplyBlockMethods.local.*/ = 1
def foo/*<-example::NamedApplyBlockMethods.foo().*/(a/*<-example::NamedApplyBlockMethods.foo().(a)*/: Int/*->scala::Int#*/ = 1, b/*<-example::NamedApplyBlockMethods.foo().(b)*/: Int/*->scala::Int#*/ = 2, c/*<-example::NamedApplyBlockMethods.foo().(c)*/: Int/*->scala::Int#*/ = 3): Int/*->scala::Int#*/ = a/*->example::NamedApplyBlockMethods.foo().(a)*/ +/*->scala::Int#`+`(+4).*/ b/*->example::NamedApplyBlockMethods.foo().(b)*/ +/*->scala::Int#`+`(+4).*/ c/*->example::NamedApplyBlockMethods.foo().(c)*/
def baseCase/*<-example::NamedApplyBlockMethods.baseCase().*/ = foo/*->example::NamedApplyBlockMethods.foo().*//*->local0*/(local/*->example::NamedApplyBlockMethods.local.*/, c/*->example::NamedApplyBlockMethods.foo().(c)*/ = 3)
def recursive/*<-example::NamedApplyBlockMethods.recursive().*/ = foo/*->example::NamedApplyBlockMethods.foo().*//*->local2*/(local/*->example::NamedApplyBlockMethods.local.*/, c/*->example::NamedApplyBlockMethods.foo().(c)*/ = foo/*->example::NamedApplyBlockMethods.foo().*//*->local3*/(local/*->example::NamedApplyBlockMethods.local.*/, c/*->example::NamedApplyBlockMethods.foo().(c)*/ = 3))
def baseCase/*<-example::NamedApplyBlockMethods.baseCase().*/ = foo/*->example::NamedApplyBlockMethods.foo().*/(local/*->example::NamedApplyBlockMethods.local.*/, c/*->example::NamedApplyBlockMethods.foo().(c)*/ = 3)
def recursive/*<-example::NamedApplyBlockMethods.recursive().*/ = foo/*->example::NamedApplyBlockMethods.foo().*//*->local1*/(local/*->example::NamedApplyBlockMethods.local.*/, c/*->example::NamedApplyBlockMethods.foo().(c)*/ = foo/*->example::NamedApplyBlockMethods.foo().*/(local/*->example::NamedApplyBlockMethods.local.*/, c/*->example::NamedApplyBlockMethods.foo().(c)*/ = 3))
}

object NamedApplyBlockCaseClassConstruction/*<-example::NamedApplyBlockCaseClassConstruction.*/ {
case class Msg/*<-example::NamedApplyBlockCaseClassConstruction.Msg#*/(body/*<-example::NamedApplyBlockCaseClassConstruction.Msg#body.*/: String/*->scala::Predef.String#*/, head/*<-example::NamedApplyBlockCaseClassConstruction.Msg#head.*/: String/*->scala::Predef.String#*/ = "default", tail/*<-example::NamedApplyBlockCaseClassConstruction.Msg#tail.*/: String/*->scala::Predef.String#*/)
val bodyText/*<-example::NamedApplyBlockCaseClassConstruction.bodyText.*/ = "body"
val msg/*<-example::NamedApplyBlockCaseClassConstruction.msg.*/ = Msg/*->example::NamedApplyBlockCaseClassConstruction.Msg.*//*->local4*//*->example::NamedApplyBlockCaseClassConstruction.Msg.apply().*/(bodyText/*->example::NamedApplyBlockCaseClassConstruction.bodyText.*/, tail/*->example::NamedApplyBlockCaseClassConstruction.Msg.apply().(tail)*/ = "tail")
val msg/*<-example::NamedApplyBlockCaseClassConstruction.msg.*/ = Msg/*->example::NamedApplyBlockCaseClassConstruction.Msg.*//*->example::NamedApplyBlockCaseClassConstruction.Msg.apply().*/(bodyText/*->example::NamedApplyBlockCaseClassConstruction.bodyText.*/, tail/*->example::NamedApplyBlockCaseClassConstruction.Msg.apply().(tail)*/ = "tail")
}
16 changes: 5 additions & 11 deletions tests/semanticdb/metac.expect
Original file line number Diff line number Diff line change
Expand Up @@ -2115,8 +2115,8 @@ Schema => SemanticDB v4
Uri => NamedApplyBlock.scala
Text => empty
Language => Scala
Symbols => 46 entries
Occurrences => 46 entries
Symbols => 43 entries
Occurrences => 43 entries

Symbols:
example/NamedApplyBlockCaseClassConstruction. => final object NamedApplyBlockCaseClassConstruction
Expand Down Expand Up @@ -2160,11 +2160,8 @@ example/NamedApplyBlockMethods.foo().(b) => param b
example/NamedApplyBlockMethods.foo().(c) => param c
example/NamedApplyBlockMethods.local. => val method local
example/NamedApplyBlockMethods.recursive(). => method recursive
local0 => val local b$1
local1 => val local c$1
local2 => val local b$3
local3 => val local b$2
local4 => val local head$1
local0 => val local c$1
local1 => val local b$1

Occurrences:
[0:8..0:15): example <- example/
Expand All @@ -2185,16 +2182,14 @@ Occurrences:
[4:61..4:62): c -> example/NamedApplyBlockMethods.foo().(c)
[5:6..5:14): baseCase <- example/NamedApplyBlockMethods.baseCase().
[5:17..5:20): foo -> example/NamedApplyBlockMethods.foo().
[5:17..5:17): -> local0
[5:21..5:26): local -> example/NamedApplyBlockMethods.local.
[5:28..5:29): c -> example/NamedApplyBlockMethods.foo().(c)
[6:6..6:15): recursive <- example/NamedApplyBlockMethods.recursive().
[6:18..6:21): foo -> example/NamedApplyBlockMethods.foo().
[6:18..6:18): -> local2
[6:18..6:18): -> local1
[6:22..6:27): local -> example/NamedApplyBlockMethods.local.
[6:29..6:30): c -> example/NamedApplyBlockMethods.foo().(c)
[6:33..6:36): foo -> example/NamedApplyBlockMethods.foo().
[6:33..6:33): -> local3
[6:37..6:42): local -> example/NamedApplyBlockMethods.local.
[6:44..6:45): c -> example/NamedApplyBlockMethods.foo().(c)
[9:7..9:43): NamedApplyBlockCaseClassConstruction <- example/NamedApplyBlockCaseClassConstruction.
Expand All @@ -2209,7 +2204,6 @@ Occurrences:
[11:6..11:14): bodyText <- example/NamedApplyBlockCaseClassConstruction.bodyText.
[12:6..12:9): msg <- example/NamedApplyBlockCaseClassConstruction.msg.
[12:12..12:15): Msg -> example/NamedApplyBlockCaseClassConstruction.Msg.
[12:12..12:12): -> local4
[12:15..12:15): -> example/NamedApplyBlockCaseClassConstruction.Msg.apply().
[12:16..12:24): bodyText -> example/NamedApplyBlockCaseClassConstruction.bodyText.
[12:26..12:30): tail -> example/NamedApplyBlockCaseClassConstruction.Msg.apply().(tail)
Expand Down