Skip to content
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

Avoid loosing denotations of named types during integrate #22839

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
8 changes: 0 additions & 8 deletions compiler/src/dotty/tools/dotc/ast/tpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -842,14 +842,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 {
Expand Down
49 changes: 40 additions & 9 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3823,9 +3823,47 @@ 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(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 =>
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 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)

final def derivedLambdaType(paramNames: List[ThisName] = this.paramNames,
paramInfos: List[PInfo] = this.paramInfos,
resType: Type = this.resType)(using Context): This =
Expand Down Expand Up @@ -6310,14 +6348,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)

Expand Down
17 changes: 10 additions & 7 deletions compiler/src/dotty/tools/dotc/inlines/Inliner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
2 changes: 0 additions & 2 deletions tests/pos/annot-17242.scala
Original file line number Diff line number Diff line change
@@ -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)) = ()
Loading