Skip to content

Commit 2953eab

Browse files
oderskymbovel
odersky
and
mbovel
authored
Avoid blowup of compute times for ill-formed retains (#24564)
This reverts one commit in #24556 but keeps its logic. It just moves the code elsewhere. --------- Co-authored-by: Matt Bovel <matthieu@bovel.net>
1 parent 9aa08e2 commit 2953eab

File tree

4 files changed

+137
-33
lines changed

4 files changed

+137
-33
lines changed

‎compiler/src/dotty/tools/dotc/core/Annotations.scala‎

Lines changed: 72 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ import ast.tpd, tpd.*
77
import util.Spans.Span
88
import printing.{Showable, Printer}
99
import printing.Texts.Text
10+
import cc.isRetainsLike
11+
import config.Feature
12+
import Decorators.*
1013

1114
import scala.annotation.internal.sharable
1215

@@ -53,33 +56,74 @@ object Annotations {
5356
* be overridden. Returns EmptyAnnotation if type type map produces a range
5457
* type, since ranges cannot be types of trees.
5558
*/
56-
def mapWith(tm: TypeMap)(using Context) =
57-
val args = tpd.allArguments(tree)
58-
if args.isEmpty then this
59-
else
60-
// Checks if `tm` would result in any change by applying it to types
61-
// inside the annotations' arguments and checking if the resulting types
62-
// are different.
63-
val findDiff = new TreeAccumulator[Type]:
64-
def apply(x: Type, tree: Tree)(using Context): Type =
65-
if tm.isRange(x) then x
66-
else
67-
val tp1 = tm(tree.tpe)
68-
foldOver(if !tp1.exists || tp1.eql(tree.tpe) then x else tp1, tree)
69-
val diff = findDiff(NoType, args)
70-
if tm.isRange(diff) then EmptyAnnotation
71-
else if diff.exists then derivedAnnotation(tm.mapOver(tree))
72-
else this
59+
def mapWith(tm: TypeMap)(using Context): Annotation =
60+
tpd.allArguments(tree) match
61+
case Nil => this
62+
63+
case arg :: Nil if symbol.isRetainsLike =>
64+
// Use a more efficient scheme to map retains and retainsByName annotations:
65+
// 1. Map the type argument to a simple TypeTree instead of tree-mapping
66+
// the original tree. TODO Try to use this scheme for other annotations that
67+
// take only type arguments as well. We should wait until after 3.9 LTS to
68+
// do this, though.
69+
// 2. Map all skolems (?n: T) to (?n: Any), and map all recursive captures of
70+
// that are not on CapSet to `^`. Skolems and capturing types on types
71+
// other than CapSet are not allowed in a retains annotation anyway,
72+
// so the underlying type does not matter. This simplification prevents
73+
// exponential blowup in some cases. See i24556.scala and i24556a.scala.
74+
// 3. Drop the annotation entirely if CC is not enabled somehwere.
75+
76+
def sanitize(tp: Type): Type = tp match
77+
case SkolemType(_) =>
78+
SkolemType(defn.AnyType)
79+
case tp @ AnnotatedType(parent, ann)
80+
if ann.symbol.isRetainsLike && parent.typeSymbol != defn.Caps_CapSet =>
81+
tp.derivedAnnotatedType(parent, Annotation(defn.RetainsCapAnnot, ann.tree.span))
82+
case tp @ OrType(tp1, tp2) =>
83+
tp.derivedOrType(sanitize(tp1), sanitize(tp2))
84+
case _ =>
85+
tp
86+
87+
def rebuild(tree: Tree, mappedType: Type): Tree = tree match
88+
case Apply(fn, Nil) => cpy.Apply(tree)(rebuild(fn, mappedType), Nil)
89+
case TypeApply(fn, arg :: Nil) => cpy.TypeApply(tree)(fn, TypeTree(mappedType) :: Nil)
90+
case Block(Nil, expr) => rebuild(expr, mappedType)
91+
92+
if !Feature.ccEnabledSomewhere then
93+
EmptyAnnotation // strip retains-like annotations unless capture checking is enabled
94+
else
95+
val mappedType = sanitize(tm(arg.tpe))
96+
if mappedType `eql` arg.tpe then this
97+
else derivedAnnotation(rebuild(tree, mappedType))
98+
99+
case args =>
100+
// Checks if `tm` would result in any change by applying it to types
101+
// inside the annotations' arguments and checking if the resulting types
102+
// are different.
103+
val findDiff = new TreeAccumulator[Type]:
104+
def apply(x: Type, tree: Tree)(using Context): Type =
105+
if tm.isRange(x) then x
106+
else
107+
val tp1 = tm(tree.tpe)
108+
foldOver(if !tp1.exists || tp1.eql(tree.tpe) then x else tp1, tree)
109+
val diff = findDiff(NoType, args)
110+
if tm.isRange(diff) then EmptyAnnotation
111+
else if diff.exists then derivedAnnotation(tm.mapOver(tree))
112+
else this
113+
end mapWith
73114

74115
/** Does this annotation refer to a parameter of `tl`? */
75116
def refersToParamOf(tl: TermLambda)(using Context): Boolean =
76-
val args = tpd.allArguments(tree)
77-
if args.isEmpty then false
78-
else tree.existsSubTree:
79-
case id: (Ident | This) => id.tpe.stripped match
80-
case TermParamRef(tl1, _) => tl eq tl1
81-
case _ => false
117+
def isLambdaParam(t: Type) = t match
118+
case TermParamRef(tl1, _) => tl eq tl1
82119
case _ => false
120+
tpd.allArguments(tree).exists: arg =>
121+
if arg.isType then
122+
arg.tpe.existsPart(isLambdaParam, stopAt = StopAt.Static)
123+
else
124+
arg.existsSubTree:
125+
case id: (Ident | This) => isLambdaParam(id.tpe.stripped)
126+
case _ => false
83127

84128
/** A string representation of the annotation. Overridden in BodyAnnotation.
85129
*/
@@ -248,6 +292,10 @@ object Annotations {
248292
}
249293
}
250294

295+
/** An annotation that is used as a result of mapping annotations
296+
* to indicate that the resulting typemap should drop the annotation
297+
* (in derivedAnnotatedType).
298+
*/
251299
@sharable val EmptyAnnotation = Annotation(EmptyTree)
252300

253301
def ThrowsAnnotation(cls: ClassSymbol)(using Context): Annotation = {
@@ -303,7 +351,7 @@ object Annotations {
303351
case annot @ ExperimentalAnnotation(msg) => ExperimentalAnnotation(msg, annot.tree.span)
304352
}
305353
}
306-
354+
307355
object PreviewAnnotation {
308356
/** Matches and extracts the message from an instance of `@preview(msg)`
309357
* Returns `Some("")` for `@preview` with no message.

‎compiler/src/dotty/tools/dotc/core/Types.scala‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import Hashable.*
3030
import Uniques.*
3131
import collection.mutable
3232
import config.Config
33-
import config.Feature
3433
import config.Feature.sourceVersion
3534
import config.SourceVersion
3635
import annotation.{tailrec, constructorOnly}
@@ -5905,8 +5904,9 @@ object Types extends TypeUtils {
59055904

59065905
override def underlying(using Context): Type = parent
59075906

5908-
def derivedAnnotatedType(parent: Type, annot: Annotation)(using Context): AnnotatedType =
5907+
def derivedAnnotatedType(parent: Type, annot: Annotation)(using Context): Type =
59095908
if ((parent eq this.parent) && (annot eq this.annot)) this
5909+
else if annot == EmptyAnnotation then parent
59105910
else AnnotatedType(parent, annot)
59115911

59125912
override def stripTypeVar(using Context): Type =
@@ -6484,13 +6484,7 @@ object Types extends TypeUtils {
64846484
mapCapturingType(tp, parent, refs, variance)
64856485

64866486
case tp @ AnnotatedType(underlying, annot) =>
6487-
if annot.symbol.isRetainsLike && !Feature.ccEnabledSomewhere then
6488-
this(underlying) // strip retains like annotations unless capture checking is enabled
6489-
else
6490-
val underlying1 = this(underlying)
6491-
val annot1 = annot.mapWith(this)
6492-
if annot1 eq EmptyAnnotation then underlying1
6493-
else derivedAnnotatedType(tp, underlying1, annot1)
6487+
derivedAnnotatedType(tp, this(underlying), annot.mapWith(this))
64946488

64956489
case _: ThisType
64966490
| _: BoundType

‎tests/pos-custom-args/captures/i24556.scala‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import language.experimental.captureChecking
2+
3+
trait Item
4+
5+
trait ItOps[+T, +CC[_], +C]:
6+
def ++[B >: T](other: It[B]^): CC[B]^{this, other}
7+
8+
trait It[+T] extends ItOps[T, It, It[T]]
9+
10+
trait Sq[+T] extends It[T] with ItOps[T, Seq, Seq[T]]
11+
12+
def items: Sq[Item] = ???
13+
14+
@main def main(): It[Item] =
15+
items
16+
++ items
17+
++ items
18+
++ items
19+
++ items
20+
++ items
21+
++ items
22+
++ items
23+
++ items
24+
++ items
25+
++ items
26+
++ items
27+
++ items
28+
++ items
29+
++ items
30+
++ items
31+
++ items
32+
++ items
33+
++ items
34+
++ items
35+
++ items

‎tests/pos-custom-args/captures/i24556a.scala‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import language.experimental.captureChecking
2+
3+
trait Item
4+
def items : Seq[Item] = ???
5+
6+
@main def main() =
7+
items
8+
++ items
9+
++ items
10+
++ items
11+
++ items
12+
++ items
13+
++ items
14+
++ items
15+
++ items
16+
++ items
17+
++ items
18+
++ items
19+
++ items
20+
++ items
21+
++ items
22+
++ items
23+
++ items
24+
++ items
25+
++ items
26+
++ items
27+
++ items

0 commit comments

Comments
 (0)