From f6ab96da333fee5403ed4f1d0512acf933c4e247 Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Tue, 18 Mar 2025 16:01:13 +0000 Subject: [PATCH 1/5] Revert "Rename `InlineCopier` to `ConservativeTreeCopier`, use it in `TypeMap`s" This reverts commit bb63e31a8384e6e3127ee679ea0033e8437ef9ce. [Cherry-picked e01af070bd507ae2b82be9ffe46261a59a075c4c] --- compiler/src/dotty/tools/dotc/ast/tpd.scala | 8 -------- compiler/src/dotty/tools/dotc/core/Types.scala | 9 +-------- .../src/dotty/tools/dotc/inlines/Inliner.scala | 17 ++++++++++------- 3 files changed, 11 insertions(+), 23 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index 6ce8d93af76a..df220271ba4c 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -832,14 +832,6 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { Closure(tree: Tree)(env, meth, tpt) } - // This is a more fault-tolerant copier that does not cause errors when - // function types in applications are undefined. - // This was called `Inliner.InlineCopier` before 3.6.3. - class ConservativeTreeCopier() extends TypedTreeCopier: - override def Apply(tree: Tree)(fun: Tree, args: List[Tree])(using Context): Apply = - if fun.tpe.widen.exists then super.Apply(tree)(fun, args) - else untpd.cpy.Apply(tree)(fun, args).withTypeUnchecked(tree.tpe) - override def skipTransform(tree: Tree)(using Context): Boolean = tree.tpe.isError implicit class TreeOps[ThisTree <: tpd.Tree](private val tree: ThisTree) extends AnyVal { diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 6c3c8ef9fe85..994179649704 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -5939,14 +5939,7 @@ object Types extends TypeUtils { } } - private def treeTypeMap = new TreeTypeMap( - typeMap = this, - // Using `ConservativeTreeCopier` is needed when copying dependent annoted - // types, where we can refer to a previous parameter represented as - // `TermParamRef` that has no underlying type yet. - // See tests/pos/annot-17242.scala. - cpy = ConservativeTreeCopier() - ) + private def treeTypeMap = new TreeTypeMap(typeMap = this) def mapOver(syms: List[Symbol]): List[Symbol] = mapSymbols(syms, treeTypeMap) diff --git a/compiler/src/dotty/tools/dotc/inlines/Inliner.scala b/compiler/src/dotty/tools/dotc/inlines/Inliner.scala index 50eaa0f57e38..63a99104685f 100644 --- a/compiler/src/dotty/tools/dotc/inlines/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/inlines/Inliner.scala @@ -96,6 +96,15 @@ object Inliner: } end isElideableExpr + // InlineCopier is a more fault-tolerant copier that does not cause errors when + // function types in applications are undefined. This is necessary since we copy at + // the same time as establishing the proper context in which the copied tree should + // be evaluated. This matters for opaque types, see neg/i14653.scala. + private class InlineCopier() extends TypedTreeCopier: + override def Apply(tree: Tree)(fun: Tree, args: List[Tree])(using Context): Apply = + if fun.tpe.widen.exists then super.Apply(tree)(fun, args) + else untpd.cpy.Apply(tree)(fun, args).withTypeUnchecked(tree.tpe) + // InlinerMap is a TreeTypeMap with special treatment for inlined arguments: // They are generally left alone (not mapped further, and if they wrap a type // the type Inlined wrapper gets dropped. @@ -108,13 +117,7 @@ object Inliner: substFrom: List[Symbol], substTo: List[Symbol])(using Context) extends TreeTypeMap( - typeMap, treeMap, oldOwners, newOwners, substFrom, substTo, - // It is necessary to use the `ConservativeTreeCopier` since we copy at - // the same time as establishing the proper context in which the copied - // tree should be evaluated. This matters for opaque types, see - // neg/i14653.scala. - ConservativeTreeCopier() - ): + typeMap, treeMap, oldOwners, newOwners, substFrom, substTo, InlineCopier()): override def transform(tree: Tree)(using Context): Tree = tree match From 88ef51c697a6c2fd67830d719787ad038f1df72b Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Wed, 19 Mar 2025 15:35:57 +0000 Subject: [PATCH 2/5] Avoid loosing denotations of named types during `integrate` --- .../src/dotty/tools/dotc/core/Types.scala | 28 ++++++++++++++++++- tests/pos/annot-17242.scala | 2 -- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 994179649704..59049712e52c 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -3730,9 +3730,35 @@ object Types extends TypeUtils { def integrate(tparams: List[ParamInfo], tp: Type)(using Context): Type = (tparams: @unchecked) match { case LambdaParam(lam, _) :: _ => tp.subst(lam, this) // This is where the precondition is necessary. - case params: List[Symbol @unchecked] => tp.subst(params, paramRefs) + case params: List[Symbol @unchecked] => IntegrateMap(params, paramRefs)(tp) } + /** A map that replaces references to symbols in `params` by the types in + * `paramRefs`. + * + * It is similar to [[Substituters#subst]] but avoids reloading denotations + * of named types by overriding `derivedSelect`. + * + * This is needed because during integration, [[TermParamRef]]s refer to a + * [[LambdaType]] that is not yet fully constructed, in particular for wich + * `paramInfos` is `null`. In that case all [[TermParamRef]]s have + * [[NoType]] as underlying type. Reloading denotions of selections + * involving such [[TermParamRef]]s in [[NamedType#withPrefix]] could then + * result in a [[NoDenotation]], which would make later disambiguation of + * overloads impossible. See `tests/pos/annot-17242.scala` for example. + */ + private class IntegrateMap(params: List[Symbol], paramRefs: List[Type])(using Context) extends TypeMap: + override def apply(tp: Type) = + tp match + case tp: NamedType if params.contains(tp.symbol) => paramRefs(params.indexOf(tp.symbol)) + case _: BoundType | _: ThisType => tp + case _ => mapOver(tp) + + override def derivedSelect(tp: NamedType, pre: Type): Type = + if tp.prefix eq pre then tp + else if tp.symbol.exists then NamedType(pre, tp.name, tp.denot.asSeenFrom(pre)) + else NamedType(pre, tp.name) + final def derivedLambdaType(paramNames: List[ThisName] = this.paramNames, paramInfos: List[PInfo] = this.paramInfos, resType: Type = this.resType)(using Context): This = diff --git a/tests/pos/annot-17242.scala b/tests/pos/annot-17242.scala index a8fcc9dbe15f..a1a62638ced3 100644 --- a/tests/pos/annot-17242.scala +++ b/tests/pos/annot-17242.scala @@ -1,5 +1,3 @@ -// See also tests/pos/annot-21595.scala - class local(predicate: Int) extends annotation.StaticAnnotation def failing1(x: Int, z: Int @local(x + x)) = () From 98c9ff67e36266dcbfde23b6b54c8b6af81f163b Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Mon, 5 May 2025 11:28:42 +0200 Subject: [PATCH 3/5] Avoid loosing denotations of named types during `integrate` [Cherry-picked 2f285a20140b3e63a36affbe272a7cb0a8c844a6][modified] From 7587326508cb0623d364d47f6fad3dbba5d46f46 Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Wed, 9 Apr 2025 12:13:53 +0000 Subject: [PATCH 4/5] Copy implementation of SubstMap to IntegrateMap --- compiler/src/dotty/tools/dotc/core/Types.scala | 18 +++++++++++++++--- tests/neg/i12448.scala | 2 +- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 59049712e52c..b98670e48c33 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -3747,14 +3747,26 @@ object Types extends TypeUtils { * result in a [[NoDenotation]], which would make later disambiguation of * overloads impossible. See `tests/pos/annot-17242.scala` for example. */ - private class IntegrateMap(params: List[Symbol], paramRefs: List[Type])(using Context) extends TypeMap: + private class IntegrateMap(from: List[Symbol], to: List[Type])(using Context) extends TypeMap: override def apply(tp: Type) = + // Same implementation as in `SubstMap`, except the `derivedSelect` in + // the `NamedType` case, and the default case that just calls `mapOver`. tp match - case tp: NamedType if params.contains(tp.symbol) => paramRefs(params.indexOf(tp.symbol)) + case tp: NamedType => + val sym = tp.symbol + var fs = from + var ts = to + while (fs.nonEmpty && ts.nonEmpty) { + if (fs.head eq sym) return ts.head + fs = fs.tail + ts = ts.tail + } + if (tp.prefix `eq` NoPrefix) tp + else derivedSelect(tp, apply(tp.prefix)) case _: BoundType | _: ThisType => tp case _ => mapOver(tp) - override def derivedSelect(tp: NamedType, pre: Type): Type = + override final def derivedSelect(tp: NamedType, pre: Type): Type = if tp.prefix eq pre then tp else if tp.symbol.exists then NamedType(pre, tp.name, tp.denot.asSeenFrom(pre)) else NamedType(pre, tp.name) diff --git a/tests/neg/i12448.scala b/tests/neg/i12448.scala index e495cfd19f1d..df9ee94ecbfe 100644 --- a/tests/neg/i12448.scala +++ b/tests/neg/i12448.scala @@ -1,5 +1,5 @@ object Main { def mkArray[T <: A]: T#AType // error // error - mkArray[Array] // was: "assertion failed: invalid prefix HKTypeLambda..." + mkArray[Array] val x = mkArray[Array] } From 5133a0285f700805de3e4085be08a1f93bc4bbd8 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Mon, 5 May 2025 11:30:50 +0200 Subject: [PATCH 5/5] Copy implementation of SubstMap to IntegrateMap [Cherry-picked 9bef68c5ddf0f30b83d2daadbe8e735ce19f787f][modified]