From 50dc05c334e0831a99386b0197d9939ee4991e52 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 5 Mar 2021 10:01:18 +0100 Subject: [PATCH 1/4] Make `overloaded` in `Quotes` return a Term instead of an `apply`. It calls `applyOverloaded`, but the interaction of named and default parameters means that sometimes arguments have to be lifted out and a block is returned instead of an apply. --- compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala | 8 ++++---- library/src/scala/quoted/Quotes.scala | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala b/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala index 89eb924598c1..d23e40796034 100644 --- a/compiler/src/scala/quoted/runtime/impl/QuotesImpl.scala +++ b/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) = diff --git a/library/src/scala/quoted/Quotes.scala b/library/src/scala/quoted/Quotes.scala index 31c840e49578..a214c16bf1dd 100644 --- a/library/src/scala/quoted/Quotes.scala +++ b/library/src/scala/quoted/Quotes.scala @@ -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 From f35c521ddc1eb43327c62c6f7958fd4fa3822922 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 5 Mar 2021 11:07:29 +0100 Subject: [PATCH 2/4] Change order of evaluation for default parameters Explicitly given parameters are always evaluated before default parameters. Aligns with spec and Scala 2. --- compiler/src/dotty/tools/dotc/ast/tpd.scala | 4 ++-- compiler/src/dotty/tools/dotc/typer/Applications.scala | 4 ++-- compiler/test/dotc/pos-test-pickling.blacklist | 3 +++ tests/run/i11571.check | 2 ++ tests/run/i11571.scala | 8 ++++++++ 5 files changed, 17 insertions(+), 4 deletions(-) create mode 100644 tests/run/i11571.check create mode 100644 tests/run/i11571.scala diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index 60201ae532a0..b53c3d390371 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -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) } diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index aef0d8421afb..7e3c9708e35e 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -756,7 +756,7 @@ trait Applications extends Compatibility { 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) then // need to lift arguments to maintain evaluation order in the // presence of argument reorderings. @@ -787,7 +787,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) diff --git a/compiler/test/dotc/pos-test-pickling.blacklist b/compiler/test/dotc/pos-test-pickling.blacklist index 7e1e1a8b34ec..c770c6cd1866 100644 --- a/compiler/test/dotc/pos-test-pickling.blacklist +++ b/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 diff --git a/tests/run/i11571.check b/tests/run/i11571.check new file mode 100644 index 000000000000..c4a9d141e4b6 --- /dev/null +++ b/tests/run/i11571.check @@ -0,0 +1,2 @@ +17 +42 diff --git a/tests/run/i11571.scala b/tests/run/i11571.scala new file mode 100644 index 000000000000..5bd6af3336e0 --- /dev/null +++ b/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) +} \ No newline at end of file From c701cbb9421be67e9159403c2d50a5003cb16314 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 5 Mar 2021 12:17:42 +0100 Subject: [PATCH 3/4] Avoid some argument reorderings Avoid reordering arguments if all explicitly given arguments are pure. In that case they can be freely interspersed with default getters. --- .../src/dotty/tools/dotc/typer/Applications.scala | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 7e3c9708e35e..2df78186c057 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/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) && !isJavaAnnotConstr(methRef.symbol) then + 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. From 1f27a2553096ba180f5bf07dddab0ef147040829 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 5 Mar 2021 16:32:37 +0100 Subject: [PATCH 4/4] Update semanticDB expectations --- .../expect/NamedApplyBlock.expect.scala | 6 +++--- tests/semanticdb/metac.expect | 16 +++++----------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/tests/semanticdb/expect/NamedApplyBlock.expect.scala b/tests/semanticdb/expect/NamedApplyBlock.expect.scala index ef438d0e74f6..db1311082bad 100644 --- a/tests/semanticdb/expect/NamedApplyBlock.expect.scala +++ b/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") } diff --git a/tests/semanticdb/metac.expect b/tests/semanticdb/metac.expect index ea69b3755ea7..cef74b66e44f 100644 --- a/tests/semanticdb/metac.expect +++ b/tests/semanticdb/metac.expect @@ -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 @@ -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/ @@ -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. @@ -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)