Merge pull request #11609 from dotty-staging/fix-11571

Change order of evaluation for default parameters

Author: nicolasstucki

compiler/src/dotty/tools/dotc/ast/tpd.scala

@@ -825,8 +825,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)
         }

compiler/src/dotty/tools/dotc/typer/Applications.scala

@@ -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.
 
@@ -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)

compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala

@@ -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) =

compiler/test/dotc/pos-test-pickling.blacklist

@@ -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

library/src/scala/quoted/Quotes.scala

@@ -783,10 +783,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
 

tests/run/i11571.check

@@ -0,0 +1,2 @@
+17
+42

tests/run/i11571.scala

@@ -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)
+}

tests/semanticdb/expect/NamedApplyBlock.expect.scala

@@ -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")
 }

tests/semanticdb/metac.expect

@@ -2157,8 +2157,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
@@ -2202,11 +2202,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/
@@ -2227,16 +2224,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.
@@ -2251,7 +2246,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)