Skip to content

Commit b30f2dd

Browse files
committed
apply box adaptation when checking the compatibility of overrides
1 parent 5860250 commit b30f2dd

File tree

6 files changed

+95
-27
lines changed

6 files changed

+95
-27
lines changed

compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -141,25 +141,12 @@ class CheckCaptures extends Recheck, SymTransformer:
141141

142142
override def run(using Context): Unit =
143143
if Feature.ccEnabled then
144-
checkOverrides.traverse(ctx.compilationUnit.tpdTree)
145144
super.run
146145

147146
override def transformSym(sym: SymDenotation)(using Context): SymDenotation =
148147
if Synthetics.needsTransform(sym) then Synthetics.transformFromCC(sym)
149148
else super.transformSym(sym)
150149

151-
/** Check overrides again, taking capture sets into account.
152-
* TODO: Can we avoid doing overrides checks twice?
153-
* We need to do them here since only at this phase CaptureTypes are relevant
154-
* But maybe we can then elide the check during the RefChecks phase under captureChecking?
155-
*/
156-
def checkOverrides = new TreeTraverser:
157-
def traverse(t: Tree)(using Context) =
158-
t match
159-
case t: Template => checkAllOverrides(ctx.owner.asClass)
160-
case _ =>
161-
traverseChildren(t)
162-
163150
class CaptureChecker(ictx: Context) extends Rechecker(ictx):
164151
import ast.tpd.*
165152

@@ -668,8 +655,11 @@ class CheckCaptures extends Recheck, SymTransformer:
668655
case _ =>
669656
expected
670657

671-
/** Adapt `actual` type to `expected` type by inserting boxing and unboxing conversions */
672-
def adaptBoxed(actual: Type, expected: Type, pos: SrcPos)(using Context): Type =
658+
/** Adapt `actual` type to `expected` type by inserting boxing and unboxing conversions
659+
*
660+
* @param alwaysConst always make capture set variables constant after adaptation
661+
*/
662+
def adaptBoxed(actual: Type, expected: Type, pos: SrcPos, alwaysConst: Boolean = false)(using Context): Type =
673663

674664
/** Adapt function type `actual`, which is `aargs -> ares` (possibly with dependencies)
675665
* to `expected` type.
@@ -806,9 +796,9 @@ class CheckCaptures extends Recheck, SymTransformer:
806796
}
807797
if !insertBox then // unboxing
808798
markFree(criticalSet, pos)
809-
recon(CapturingType(parent1, cs1, !actualIsBoxed))
799+
recon(CapturingType(parent1, if alwaysConst then CaptureSet(cs1.elems) else cs1, !actualIsBoxed))
810800
else
811-
recon(CapturingType(parent1, cs1, actualIsBoxed))
801+
recon(CapturingType(parent1, if alwaysConst then CaptureSet(cs1.elems) else cs1, actualIsBoxed))
812802
}
813803

814804
var actualw = actual.widenDealias
@@ -827,7 +817,38 @@ class CheckCaptures extends Recheck, SymTransformer:
827817
else actual
828818
end adaptBoxed
829819

820+
/** Check overrides again, taking capture sets into account.
821+
* TODO: Can we avoid doing overrides checks twice?
822+
* We need to do them here since only at this phase CaptureTypes are relevant
823+
* But maybe we can then elide the check during the RefChecks phase under captureChecking?
824+
*/
825+
def checkOverrides = new TreeTraverser:
826+
/** Check subtype with box adaptation.
827+
* This function is passed to RefChecks to check the compatibility of overriding pairs.
828+
* @param sym symbol of the field definition that is being checked
829+
*/
830+
def checkSubtype(sym: Symbol)(tree: Tree, actual: Type, expected: Type)(using Context): Boolean =
831+
val expected1 = alignDependentFunction(addOuterRefs(expected, actual), actual.stripCapturing)
832+
val actual1 =
833+
val saved = curEnv
834+
try
835+
curEnv = Env(sym, nestedInOwner = false, CaptureSet.Var(), isBoxed = false, outer0 = curEnv)
836+
adaptBoxed(actual, expected1, tree.srcPos, alwaysConst = true) match
837+
case tp @ CapturingType(parent, refs) =>
838+
CapturingType(parent, CaptureSet(refs.elems))
839+
case tp => tp
840+
finally curEnv = saved
841+
actual1 frozen_<:< expected1
842+
843+
def traverse(t: Tree)(using Context) =
844+
t match
845+
case t: Template =>
846+
checkAllOverrides(ctx.owner.asClass, isSubType = sym => (tp1, tp2) => checkSubtype(sym)(t, tp1, tp2))
847+
case _ =>
848+
traverseChildren(t)
849+
830850
override def checkUnit(unit: CompilationUnit)(using Context): Unit =
851+
checkOverrides.traverse(unit.tpdTree)
831852
Setup(preRecheckPhase, thisPhase, recheckDef)
832853
.traverse(ctx.compilationUnit.tpdTree)
833854
//println(i"SETUP:\n${Recheck.addRecheckedTypes.transform(ctx.compilationUnit.tpdTree)}")

compiler/src/dotty/tools/dotc/typer/RefChecks.scala

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,16 @@ object RefChecks {
362362
* Type members are always assumed to match.
363363
*/
364364
def trueMatch: Boolean =
365-
member.isType || memberTp(self).matches(otherTp(self))
365+
member.isType || withMode(Mode.IgnoreCaptures) {
366+
// `matches` does not perform box adaptation so the result here would be
367+
// spurious during capture checking.
368+
//
369+
// Instead of parameterizing `matches` with the function for subtype checking
370+
// with box adaptation, we simply ignore capture annotations here.
371+
// This should be safe since the compatibility under box adaptation is already
372+
// checked.
373+
memberTp(self).matches(otherTp(self))
374+
}
366375

367376
def emitOverrideError(fullmsg: Message) =
368377
if (!(hasErrors && member.is(Synthetic) && member.is(Module))) {

tests/neg-custom-args/captures/lazylists2.check

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,3 @@
1-
-- [E164] Declaration Error: tests/neg-custom-args/captures/lazylists2.scala:50:10 -------------------------------------
2-
50 | def tail: {xs, f} LazyList[B] = xs.tail.map(f) // error
3-
| ^
4-
| error overriding method tail in trait LazyList of type -> {Mapped.this} LazyList[B];
5-
| method tail of type -> {xs, f} LazyList[B] has incompatible type
6-
|
7-
| longer explanation available when compiling with `-explain`
81
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazylists2.scala:18:4 ------------------------------------
92
18 | final class Mapped extends LazyList[B]: // error
103
| ^
@@ -37,6 +30,18 @@
3730
41 | def tail: {this} LazyList[B] = xs.tail.map(f) // error
3831
| ^
3932
|(f : A => B) cannot be referenced here; it is not included in the allowed capture set {xs} of the self type of class Mapped
33+
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazylists2.scala:45:4 ------------------------------------
34+
45 | final class Mapped extends LazyList[B]: // error
35+
| ^
36+
| Found: {f, xs} LazyList[? B]
37+
| Required: {xs} LazyList[B]
38+
46 | this: ({xs, f} Mapped) =>
39+
47 | def isEmpty = false
40+
48 | def head: B = f(xs.head)
41+
49 | def tail: {xs, f} LazyList[B] = xs.tail.map(f)
42+
50 | new Mapped
43+
|
44+
| longer explanation available when compiling with `-explain`
4045
-- Error: tests/neg-custom-args/captures/lazylists2.scala:60:10 --------------------------------------------------------
4146
60 | class Mapped2 extends Mapped: // error
4247
| ^

tests/neg-custom-args/captures/lazylists2.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,12 @@ extension [A](xs: {*} LazyList[A])
4242
new Mapped
4343

4444
def map4[B](f: A => B): {xs} LazyList[B] =
45-
final class Mapped extends LazyList[B]:
45+
final class Mapped extends LazyList[B]: // error
4646
this: ({xs, f} Mapped) =>
4747

4848
def isEmpty = false
4949
def head: B = f(xs.head)
50-
def tail: {xs, f} LazyList[B] = xs.tail.map(f) // error
50+
def tail: {xs, f} LazyList[B] = xs.tail.map(f)
5151
new Mapped
5252

5353
def map5[B](f: A => B): LazyList[B] =
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import language.experimental.captureChecking
2+
3+
abstract class A[X] {
4+
def foo(x: X): X
5+
}
6+
7+
class IO
8+
class C
9+
10+
def test(io: {*} IO) = {
11+
class B extends A[{io} C] { // X =:= {io} C // error
12+
override def foo(x: {io} C): {io} C = ??? // error
13+
}
14+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import language.experimental.captureChecking
2+
3+
class IO
4+
5+
abstract class A[X, Y] {
6+
def foo(x: Unit): X
7+
def bar(x: Int, y: {} IO): X
8+
def baz(x: Y): X
9+
}
10+
11+
class C
12+
13+
def test(io: {*} IO) = {
14+
class B extends A[{io} C, {} C] { // X =:= {io} C
15+
override def foo(x: Unit): {io} C = ???
16+
override def bar(x: Int, y: {} IO): {io} C = ???
17+
override def baz(x: {} C): {io} C = ???
18+
}
19+
}

0 commit comments

Comments
 (0)