From f34ff5d16319420edba7297e12ddb525487b84d8 Mon Sep 17 00:00:00 2001 From: Yichen Xu Date: Mon, 1 Apr 2024 13:07:13 +0100 Subject: [PATCH 1/4] Drop retains annotations in inferred type trees --- compiler/src/dotty/tools/dotc/cc/CaptureOps.scala | 10 ++++++++++ compiler/src/dotty/tools/dotc/typer/Typer.scala | 9 +++++++-- tests/pos-custom-args/captures/tablediff.scala | 11 +++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 tests/pos-custom-args/captures/tablediff.scala diff --git a/compiler/src/dotty/tools/dotc/cc/CaptureOps.scala b/compiler/src/dotty/tools/dotc/cc/CaptureOps.scala index 7c75ed833945..5c0dbd8508bf 100644 --- a/compiler/src/dotty/tools/dotc/cc/CaptureOps.scala +++ b/compiler/src/dotty/tools/dotc/cc/CaptureOps.scala @@ -445,6 +445,16 @@ extension (tp: AnnotatedType) case ann: CaptureAnnotation => ann.boxed case _ => false +class CleanupRetains(using Context) extends TypeMap: + def apply(tp: Type): Type = cleanupRetains(tp, this) + +/** Drop retains annotations in the type. */ +def cleanupRetains(tp: Type, theMap: CleanupRetains | Null = null)(using Context): Type = + def mapOver = (if theMap != null then theMap else new CleanupRetains).mapOver(tp) + tp match + case RetainingType(tp, _) => tp + case _ => mapOver + /** An extractor for `caps.reachCapability(ref)`, which is used to express a reach * capability as a tree in a @retains annotation. */ diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 0b05bcd078ff..7a82ac78b75c 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -48,7 +48,7 @@ import staging.StagingLevel import reporting.* import Nullables.* import NullOpsDecorator.* -import cc.{CheckCaptures, isRetainsLike} +import cc.{CheckCaptures, isRetainsLike, cleanupRetains} import config.Config import config.MigrationVersion @@ -2187,7 +2187,12 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer errorTree(tree, em"Something's wrong: missing original symbol for type tree") } case _ => - completeTypeTree(InferredTypeTree(), pt, tree) + val pt1 = cleanupRetains(pt) + // Cleans up retains annotations in inferred type trees. This is needed because + // during the typer, it is infeasible to correctly infer the capture sets in most + // cases, resulting ill-formed capture sets that could crash the pickler later on. + // See #20035. + completeTypeTree(InferredTypeTree(), pt1, tree) def typedInLambdaTypeTree(tree: untpd.InLambdaTypeTree, pt: Type)(using Context): Tree = val tp = diff --git a/tests/pos-custom-args/captures/tablediff.scala b/tests/pos-custom-args/captures/tablediff.scala new file mode 100644 index 000000000000..244ee1a46a23 --- /dev/null +++ b/tests/pos-custom-args/captures/tablediff.scala @@ -0,0 +1,11 @@ +import language.experimental.captureChecking + +trait Seq[+A]: + def zipAll[A1 >: A, B](that: Seq[B]^, thisElem: A1, thatElem: B): Seq[(A1, B)]^{this, that} + def map[B](f: A => B): Seq[B]^{this, f} + +def zipAllOption[X](left: Seq[X], right: Seq[X]) = + left.map(Option(_)).zipAll(right.map(Option(_)), None, None) + +def fillRow[T](headRow: Seq[T], tailRow: Seq[T]) = + val paddedZip = zipAllOption(headRow, tailRow) From e6e01a3720f548ab28ea4cada4fb9322640ffb76 Mon Sep 17 00:00:00 2001 From: Yichen Xu Date: Thu, 4 Apr 2024 17:23:46 +0200 Subject: [PATCH 2/4] Cleanup retains annotations in PostTyper --- compiler/src/dotty/tools/dotc/cc/Setup.scala | 2 +- .../dotty/tools/dotc/transform/PostTyper.scala | 16 ++++++++++++++-- compiler/src/dotty/tools/dotc/typer/Typer.scala | 7 +------ tests/neg-custom-args/captures/byname.check | 2 +- 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/cc/Setup.scala b/compiler/src/dotty/tools/dotc/cc/Setup.scala index 9ab41859f170..b9e25a84fc38 100644 --- a/compiler/src/dotty/tools/dotc/cc/Setup.scala +++ b/compiler/src/dotty/tools/dotc/cc/Setup.scala @@ -454,7 +454,7 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI: case _ => false def signatureChanges = - tree.tpt.hasRememberedType && !sym.isConstructor || paramSignatureChanges + (tree.tpt.hasRememberedType || tree.tpt.isInstanceOf[InferredTypeTree]) && !sym.isConstructor || paramSignatureChanges // Replace an existing symbol info with inferred types where capture sets of // TypeParamRefs and TermParamRefs put in correspondence by BiTypeMaps with the diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala index 3bcec80b5b10..db451c64650a 100644 --- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala @@ -19,6 +19,7 @@ import config.Feature import util.SrcPos import reporting.* import NameKinds.WildcardParamName +import cc.* object PostTyper { val name: String = "posttyper" @@ -279,6 +280,17 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase => if !tree.symbol.is(Package) then tree else errorTree(tree, em"${tree.symbol} cannot be used as a type") + // Cleans up retains annotations in inferred type trees. This is needed because + // during the typer, it is infeasible to correctly infer the capture sets in most + // cases, resulting ill-formed capture sets that could crash the pickler later on. + // See #20035. + private def cleanupRetainsAnnot(symbol: Symbol, tpt: Tree)(using Context): Tree = + tpt match + case tpt: InferredTypeTree if !symbol.allOverriddenSymbols.hasNext => + val tpe1 = cleanupRetains(tpt.tpe) + tpt.withType(tpe1) + case _ => tpt + override def transform(tree: Tree)(using Context): Tree = try tree match { // TODO move CaseDef case lower: keep most probable trees first for performance @@ -388,7 +400,7 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase => registerIfHasMacroAnnotations(tree) checkErasedDef(tree) Checking.checkPolyFunctionType(tree.tpt) - val tree1 = cpy.ValDef(tree)(rhs = normalizeErasedRhs(tree.rhs, tree.symbol)) + val tree1 = cpy.ValDef(tree)(tpt = cleanupRetainsAnnot(tree.symbol, tree.tpt), rhs = normalizeErasedRhs(tree.rhs, tree.symbol)) if tree1.removeAttachment(desugar.UntupledParam).isDefined then checkStableSelection(tree.rhs) processValOrDefDef(super.transform(tree1)) @@ -398,7 +410,7 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase => checkErasedDef(tree) Checking.checkPolyFunctionType(tree.tpt) annotateContextResults(tree) - val tree1 = cpy.DefDef(tree)(rhs = normalizeErasedRhs(tree.rhs, tree.symbol)) + val tree1 = cpy.DefDef(tree)(tpt = cleanupRetainsAnnot(tree.symbol, tree.tpt), rhs = normalizeErasedRhs(tree.rhs, tree.symbol)) processValOrDefDef(superAcc.wrapDefDef(tree1)(super.transform(tree1).asInstanceOf[DefDef])) case tree: TypeDef => registerIfHasMacroAnnotations(tree) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 7a82ac78b75c..39afb1e73b1d 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2187,12 +2187,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer errorTree(tree, em"Something's wrong: missing original symbol for type tree") } case _ => - val pt1 = cleanupRetains(pt) - // Cleans up retains annotations in inferred type trees. This is needed because - // during the typer, it is infeasible to correctly infer the capture sets in most - // cases, resulting ill-formed capture sets that could crash the pickler later on. - // See #20035. - completeTypeTree(InferredTypeTree(), pt1, tree) + completeTypeTree(InferredTypeTree(), pt, tree) def typedInLambdaTypeTree(tree: untpd.InLambdaTypeTree, pt: Type)(using Context): Tree = val tp = diff --git a/tests/neg-custom-args/captures/byname.check b/tests/neg-custom-args/captures/byname.check index 226bee2cd0e5..c54fe7d4208e 100644 --- a/tests/neg-custom-args/captures/byname.check +++ b/tests/neg-custom-args/captures/byname.check @@ -9,7 +9,7 @@ | Found: (x$0: Int) ->{cap2} Int | Required: (x$0: Int) -> Int | - | Note that the expected type Int => Int + | Note that the expected type Int -> Int | is the previously inferred result type of method test | which is also the type seen in separately compiled sources. | The new inferred type (x$0: Int) ->{cap2} Int From 615653011e806b661ffbddb693f4887203c1adf8 Mon Sep 17 00:00:00 2001 From: Yichen Xu Date: Sun, 7 Apr 2024 23:05:01 +0200 Subject: [PATCH 3/4] Address review comments --- compiler/src/dotty/tools/dotc/cc/CaptureOps.scala | 13 +++++-------- .../src/dotty/tools/dotc/transform/PostTyper.scala | 8 ++++++-- compiler/src/dotty/tools/dotc/typer/Typer.scala | 2 +- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/cc/CaptureOps.scala b/compiler/src/dotty/tools/dotc/cc/CaptureOps.scala index 5c0dbd8508bf..b0ad8719ccfb 100644 --- a/compiler/src/dotty/tools/dotc/cc/CaptureOps.scala +++ b/compiler/src/dotty/tools/dotc/cc/CaptureOps.scala @@ -445,15 +445,12 @@ extension (tp: AnnotatedType) case ann: CaptureAnnotation => ann.boxed case _ => false -class CleanupRetains(using Context) extends TypeMap: - def apply(tp: Type): Type = cleanupRetains(tp, this) - /** Drop retains annotations in the type. */ -def cleanupRetains(tp: Type, theMap: CleanupRetains | Null = null)(using Context): Type = - def mapOver = (if theMap != null then theMap else new CleanupRetains).mapOver(tp) - tp match - case RetainingType(tp, _) => tp - case _ => mapOver +class CleanupRetains(using Context) extends TypeMap: + def apply(tp: Type): Type = + tp match + case RetainingType(tp, _) => tp + case _ => mapOver(tp) /** An extractor for `caps.reachCapability(ref)`, which is used to express a reach * capability as a tree in a @retains annotation. diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala index db451c64650a..a977694ded27 100644 --- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala @@ -286,8 +286,12 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase => // See #20035. private def cleanupRetainsAnnot(symbol: Symbol, tpt: Tree)(using Context): Tree = tpt match - case tpt: InferredTypeTree if !symbol.allOverriddenSymbols.hasNext => - val tpe1 = cleanupRetains(tpt.tpe) + case tpt: InferredTypeTree + if !symbol.allOverriddenSymbols.hasNext => + // if there are overridden symbols, the annotation comes from an explicit type of the overridden symbol + // and should be retained. + val tm = new CleanupRetains + val tpe1 = tm(tpt.tpe) tpt.withType(tpe1) case _ => tpt diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 39afb1e73b1d..0b05bcd078ff 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -48,7 +48,7 @@ import staging.StagingLevel import reporting.* import Nullables.* import NullOpsDecorator.* -import cc.{CheckCaptures, isRetainsLike, cleanupRetains} +import cc.{CheckCaptures, isRetainsLike} import config.Config import config.MigrationVersion From 98cbe060bc5cdaf241ebb26fbb57ccf2e94d50c3 Mon Sep 17 00:00:00 2001 From: Yichen Xu Date: Mon, 8 Apr 2024 17:17:16 +0200 Subject: [PATCH 4/4] Drop retained elements and keep the annotation --- compiler/src/dotty/tools/dotc/cc/CaptureOps.scala | 3 ++- compiler/src/dotty/tools/dotc/cc/Setup.scala | 2 +- tests/neg-custom-args/captures/byname.check | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/cc/CaptureOps.scala b/compiler/src/dotty/tools/dotc/cc/CaptureOps.scala index b0ad8719ccfb..deeb474f018a 100644 --- a/compiler/src/dotty/tools/dotc/cc/CaptureOps.scala +++ b/compiler/src/dotty/tools/dotc/cc/CaptureOps.scala @@ -449,7 +449,8 @@ extension (tp: AnnotatedType) class CleanupRetains(using Context) extends TypeMap: def apply(tp: Type): Type = tp match - case RetainingType(tp, _) => tp + case AnnotatedType(tp, annot) if annot.symbol == defn.RetainsAnnot || annot.symbol == defn.RetainsByNameAnnot => + RetainingType(tp, Nil, byName = annot.symbol == defn.RetainsByNameAnnot) case _ => mapOver(tp) /** An extractor for `caps.reachCapability(ref)`, which is used to express a reach diff --git a/compiler/src/dotty/tools/dotc/cc/Setup.scala b/compiler/src/dotty/tools/dotc/cc/Setup.scala index b9e25a84fc38..9ab41859f170 100644 --- a/compiler/src/dotty/tools/dotc/cc/Setup.scala +++ b/compiler/src/dotty/tools/dotc/cc/Setup.scala @@ -454,7 +454,7 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI: case _ => false def signatureChanges = - (tree.tpt.hasRememberedType || tree.tpt.isInstanceOf[InferredTypeTree]) && !sym.isConstructor || paramSignatureChanges + tree.tpt.hasRememberedType && !sym.isConstructor || paramSignatureChanges // Replace an existing symbol info with inferred types where capture sets of // TypeParamRefs and TermParamRefs put in correspondence by BiTypeMaps with the diff --git a/tests/neg-custom-args/captures/byname.check b/tests/neg-custom-args/captures/byname.check index c54fe7d4208e..e06a3a1f8268 100644 --- a/tests/neg-custom-args/captures/byname.check +++ b/tests/neg-custom-args/captures/byname.check @@ -9,7 +9,7 @@ | Found: (x$0: Int) ->{cap2} Int | Required: (x$0: Int) -> Int | - | Note that the expected type Int -> Int + | Note that the expected type Int ->{} Int | is the previously inferred result type of method test | which is also the type seen in separately compiled sources. | The new inferred type (x$0: Int) ->{cap2} Int