From 19ea6732b65291b9c32252b1d48b544f6e634fe6 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 31 Mar 2022 12:04:42 +0200 Subject: [PATCH 1/3] Flag unloadable references as errors #14783 shows an example where an implicit reference was generated that started in a type. The backend then crashed since it could not emit instructions to load that reference. We now detect such references when they are created and flag them as errors. We need to separately fix the issue that created the bad reference in the first place. --- compiler/src/dotty/tools/dotc/ast/tpd.scala | 11 +++++++---- .../src/dotty/tools/dotc/typer/Applications.scala | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index 9c4f91be3c5c..a59aefed05ac 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -404,18 +404,21 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { } /** A tree representing the same reference as the given type */ - def ref(tp: NamedType)(using Context): Tree = + def ref(tp: NamedType, needLoad: Boolean = true)(using Context): Tree = if (tp.isType) TypeTree(tp) else if (prefixIsElidable(tp)) Ident(tp) else if (tp.symbol.is(Module) && ctx.owner.isContainedIn(tp.symbol.moduleClass)) followOuterLinks(This(tp.symbol.moduleClass.asClass)) else if (tp.symbol hasAnnotation defn.ScalaStaticAnnot) Ident(tp) - else { + else val pre = tp.prefix if (pre.isSingleton) followOuterLinks(singleton(pre.dealias)).select(tp) - else Select(TypeTree(pre), tp) - } + else + val res = Select(TypeTree(pre), tp) + if needLoad && !res.symbol.isStatic then + throw new TypeError(em"cannot establish a reference to $res") + res def ref(sym: Symbol)(using Context): Tree = ref(NamedType(sym.owner.thisType, sym.name, sym.denot)) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 68c4b357b782..af5ca74dcf28 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -705,7 +705,7 @@ trait Applications extends Compatibility { def fail(msg: Message): Unit = ok = false def appPos: SrcPos = NoSourcePosition - @threadUnsafe lazy val normalizedFun: Tree = ref(methRef) + @threadUnsafe lazy val normalizedFun: Tree = ref(methRef, needLoad = false) init() } From 20cc92efc00451d3121f1860552952d78820f974 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 31 Mar 2022 13:05:33 +0200 Subject: [PATCH 2/3] Don't require loadable when trying extension methods --- compiler/src/dotty/tools/dotc/ast/tpd.scala | 10 +++++----- .../dotty/tools/dotc/interactive/Completion.scala | 12 ++++++------ .../src/dotty/tools/dotc/typer/Applications.scala | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index a59aefed05ac..fb8bd917ca20 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -413,7 +413,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { Ident(tp) else val pre = tp.prefix - if (pre.isSingleton) followOuterLinks(singleton(pre.dealias)).select(tp) + if (pre.isSingleton) followOuterLinks(singleton(pre.dealias, needLoad)).select(tp) else val res = Select(TypeTree(pre), tp) if needLoad && !res.symbol.isStatic then @@ -431,11 +431,11 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { t } - def singleton(tp: Type)(using Context): Tree = tp.dealias match { - case tp: TermRef => ref(tp) + def singleton(tp: Type, needLoad: Boolean = true)(using Context): Tree = tp.dealias match { + case tp: TermRef => ref(tp, needLoad) case tp: ThisType => This(tp.cls) - case tp: SkolemType => singleton(tp.narrow) - case SuperType(qual, _) => singleton(qual) + case tp: SkolemType => singleton(tp.narrow, needLoad) + case SuperType(qual, _) => singleton(qual, needLoad) case ConstantType(value) => Literal(value) } diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index 15628c3b83e8..ee943a0cb392 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -85,9 +85,9 @@ object Completion { * @param content The source content that we'll check the positions for the prefix * @param start The start position we'll start to look for the prefix at * @param end The end position we'll look for the prefix at - * @return Either the full prefix including the ` or an empty string + * @return Either the full prefix including the ` or an empty string */ - private def checkBacktickPrefix(content: Array[Char], start: Int, end: Int): String = + private def checkBacktickPrefix(content: Array[Char], start: Int, end: Int): String = content.lift(start) match case Some(char) if char == '`' => content.slice(start, end).mkString @@ -111,7 +111,7 @@ object Completion { // Foo.`se will result in Select(Ident(Foo), ) case (select: untpd.Select) :: _ if select.name == nme.ERROR => checkBacktickPrefix(select.source.content(), select.nameSpan.start, select.span.end) - + // import scala.util.chaining.`s will result in a Ident() case (ident: untpd.Ident) :: _ if ident.name == nme.ERROR => checkBacktickPrefix(ident.source.content(), ident.span.start, ident.span.end) @@ -177,14 +177,14 @@ object Completion { // https://github.com/com-lihaoyi/Ammonite/blob/73a874173cd337f953a3edc9fb8cb96556638fdd/amm/util/src/main/scala/ammonite/util/Model.scala private def needsBacktick(s: String) = val chunks = s.split("_", -1) - + val validChunks = chunks.zipWithIndex.forall { case (chunk, index) => chunk.forall(Chars.isIdentifierPart) || (chunk.forall(Chars.isOperatorPart) && index == chunks.length - 1 && !(chunks.lift(index - 1).contains("") && index - 1 == 0)) } - + val validStart = Chars.isIdentifierStart(s(0)) || chunks(0).forall(Chars.isOperatorPart) @@ -312,7 +312,7 @@ object Completion { /** Replaces underlying type with reduced one, when it's MatchType */ def reduceUnderlyingMatchType(qual: Tree)(using Context): Tree= - qual.tpe.widen match + qual.tpe.widen match case ctx.typer.MatchTypeInDisguise(mt) => qual.withType(mt) case _ => qual diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index af5ca74dcf28..a43d32e591ee 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -2268,7 +2268,7 @@ trait Applications extends Compatibility { case TypeApply(fun, args) => TypeApply(replaceCallee(fun, replacement), args) case _ => replacement - val methodRefTree = ref(methodRef) + val methodRefTree = ref(methodRef, needLoad = false) val truncatedSym = methodRef.symbol.asTerm.copy(info = truncateExtension(methodRef.info)) val truncatedRefTree = untpd.TypedSplice(ref(truncatedSym)).withSpan(receiver.span) val newCtx = ctx.fresh.setNewScope.setReporter(new reporting.ThrowingReporter(ctx.reporter)) From 0a95a3da491f8da73acecab4f0f7047c06c5b103 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 31 Mar 2022 19:03:48 +0200 Subject: [PATCH 3/3] Fix parameter untupling Make untupled parameters ValDefs instead of DefDefs so that their references are stable paths. Add an additional check that untupled parameters don't come from call-by-name parameters (in that case, ValDefs would be unsound). I was trying hard to allow paramater untupling for cbn parameters, but this seems to be very hard since parameter untupling happens as a desugaring step when the expected parameter type is not yet known. And it seems really hard to change from stable to unstable (or vice versa) later in the compilation pipleline without doing a lot of re-checking. Since untupled cbn parameters are a pretty extreme corner case anyway I think it's OK to just disallow them. --- .../src/dotty/tools/dotc/ast/Desugar.scala | 8 ++- .../tools/dotc/transform/PostTyper.scala | 12 ++++- tests/neg/function-arity-2.scala | 7 +++ tests/neg/i14783.scala | 3 ++ tests/pos/i14783.scala | 51 +++++++++++++++++++ tests/run/function-arity.scala | 2 +- 6 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 tests/neg/function-arity-2.scala create mode 100644 tests/neg/i14783.scala create mode 100644 tests/pos/i14783.scala diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 9cd30b94d692..de0e78bcc38d 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -39,6 +39,10 @@ object desugar { */ val MultiLineInfix: Property.Key[Unit] = Property.StickyKey() + /** An attachment key to indicate that a ValDef originated from parameter untupling. + */ + val UntupledParam: Property.Key[Unit] = Property.StickyKey() + /** What static check should be applied to a Match? */ enum MatchCheck { case None, Exhaustive, IrrefutablePatDef, IrrefutableGenFrom @@ -1426,7 +1430,9 @@ object desugar { val vdefs = params.zipWithIndex.map { case (param, idx) => - DefDef(param.name, Nil, param.tpt, selector(idx)).withSpan(param.span) + ValDef(param.name, param.tpt, selector(idx)) + .withSpan(param.span) + .withAttachment(UntupledParam, ()) } Function(param :: Nil, Block(vdefs, body)) } diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala index 926600ebbdd4..6f1e3b3f0319 100644 --- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala @@ -1,7 +1,7 @@ package dotty.tools.dotc package transform -import dotty.tools.dotc.ast.{Trees, tpd, untpd} +import dotty.tools.dotc.ast.{Trees, tpd, untpd, desugar} import scala.collection.mutable import core._ import dotty.tools.dotc.typer.Checking @@ -255,6 +255,14 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase if tree.symbol.is(ConstructorProxy) then report.error(em"constructor proxy ${tree.symbol} cannot be used as a value", tree.srcPos) + def checkStableSelection(tree: Tree)(using Context): Unit = + def check(qual: Tree) = + if !qual.tpe.isStable then + report.error(em"Parameter untupling cannot be used for call-by-name parameters", tree.srcPos) + tree match + case Select(qual, _) => check(qual) // simple select _n + case Apply(TypeApply(Select(qual, _), _), _) => check(qual) // generic select .apply[T](n) + override def transform(tree: Tree)(using Context): Tree = try tree match { // TODO move CaseDef case lower: keep most probable trees first for performance @@ -356,6 +364,8 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase case tree: ValDef => checkErasedDef(tree) val tree1 = cpy.ValDef(tree)(rhs = normalizeErasedRhs(tree.rhs, tree.symbol)) + if tree1.removeAttachment(desugar.UntupledParam).isDefined then + checkStableSelection(tree.rhs) processValOrDefDef(super.transform(tree1)) case tree: DefDef => checkErasedDef(tree) diff --git a/tests/neg/function-arity-2.scala b/tests/neg/function-arity-2.scala new file mode 100644 index 000000000000..abed11d5379b --- /dev/null +++ b/tests/neg/function-arity-2.scala @@ -0,0 +1,7 @@ +object Test { + + class T[A] { def foo(f: (=> A) => Int) = f(???) } + + def main(args: Array[String]): Unit = + new T[(Int, Int)].foo((x, y) => 0) // error // error Parameter untupling cannot be used for call-by-name parameters (twice) +} diff --git a/tests/neg/i14783.scala b/tests/neg/i14783.scala new file mode 100644 index 000000000000..d7798c3fad14 --- /dev/null +++ b/tests/neg/i14783.scala @@ -0,0 +1,3 @@ +object Test: + def foo(f: (=> (Int, Int)) => Int) = ??? + foo((a, b) => a + b) // error // error diff --git a/tests/pos/i14783.scala b/tests/pos/i14783.scala new file mode 100644 index 000000000000..1339bfc2dc6f --- /dev/null +++ b/tests/pos/i14783.scala @@ -0,0 +1,51 @@ +object Wart: + def bar(using c: Ctx)(ws: List[Wrap[c.type]]): Unit = + ws.zipWithIndex.foreach { (w, _) => w.x.foo } + +trait Wrap[C <: Ctx & Singleton]: + val ctx: C + def x: ctx.inner.X + +trait Ctx: + object inner: + type X + extension (self: X) def foo: Int = ??? + + +object WartInspector: + def myWartTraverser: WartTraverser = ??? + def inspect(using q: Quotes)(tastys: List[Tasty[q.type]]): Unit = { + val universe: WartUniverse.Aux[q.type] = WartUniverse(q) + val traverser = myWartTraverser.get(universe) + tastys.zipWithIndex.foreach { (tasty, index) => + val tree = tasty.ast + traverser.traverseTree(tree)(tree.symbol) + } + } + +object WartUniverse: + type Aux[X <: Quotes] = WartUniverse { type Q = X } + def apply[Q <: Quotes](quotes: Q): Aux[Q] = ??? + + +abstract class WartUniverse: + type Q <: Quotes + val quotes: Q + abstract class Traverser extends quotes.reflect.TreeTraverser + + +abstract class WartTraverser: + def get(u: WartUniverse): u.Traverser + +trait Tasty[Q <: Quotes & Singleton]: + val quotes: Q + def path: String + def ast: quotes.reflect.Tree + +trait Quotes: + object reflect: + type Tree + extension (self: Tree) def symbol: Symbol = ??? + type Symbol + trait TreeTraverser: + def traverseTree(tree: Tree)(symbol: Symbol): Unit \ No newline at end of file diff --git a/tests/run/function-arity.scala b/tests/run/function-arity.scala index 6d7e5bce10fb..a771bd6d3ab2 100644 --- a/tests/run/function-arity.scala +++ b/tests/run/function-arity.scala @@ -3,6 +3,6 @@ object Test { def main(args: Array[String]): Unit = { new T[(Int, Int)].foo((ii) => 0) - new T[(Int, Int)].foo((x, y) => 0) // check that this does not run into ??? + //new T[(Int, Int)].foo((x, y) => 0) // not allowed anymore } }