From 6fc8c989db5a20dbfbd015f0ccf7c6c94bd3321c Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 5 Oct 2021 11:12:27 +0200 Subject: [PATCH] Special rule for {this} in capture sets of class members Consider the lazylists.scala test in pos-custom-args/captures: ```scala class CC type Cap = {*} CC trait LazyList[+A]: this: ({*} LazyList[A]) => def isEmpty: Boolean def head: A def tail: {this} LazyList[A] object LazyNil extends LazyList[Nothing]: def isEmpty: Boolean = true def head = ??? def tail = ??? extension [A](xs: {*} LazyList[A]) def map[B](f: {*} A => B): {xs, f} LazyList[B] = class Mapped extends LazyList[B]: this: ({xs, f} Mapped) => def isEmpty = false def head: B = f(xs.head) def tail: {this} LazyList[B] = xs.tail.map(f) // OK new Mapped ``` Without this commit, the second to last line is an error since the right hand side has capture set `{xs, f}` but the required capture set is `this`. To fix this, we widen the expected type of the rhs `xs.tail.map(f)` from `{this}` to `{this, f, xs}`. That is, we add the declared captures of the self type to the expected type. The soundness argument for doing this is as follows: Since `tail` does not have parameters, the only thing it could capture are references that the receiver `this` captures as well. So `xs` and `f` must come via `this`. For instance, if the receiver `xs` of `xs.tail` happens to be pure, then `xs.tail` is pure as well. On the other hand, in the neg test `lazylists1.scala` we add the following line to `Mapped`: ```scala def concat(other: {f} LazyList[A]): {this} LazyList[A] = ??? : ({xs, f} LazyList[A]) // error ``` Here, we cannot widen the expected type from `{this}` to `{this, xs, f}` since the result of concat refers to `f` independently of `this`, namely through its parameter `other`. Hence, if `ys: {f} LazyList[String]` then ``` LazyNil.concat(ys) ``` still refers to `f` even though `LazyNil` is pure. But if we would accept the definition of `concat` above, the type of `LazyNil.concat(ys)` would be `LazyList[String]`, which is unsound. The current implementation widens the expected type of class members if the class member does not have tracked parameters. We could potentially refine this to say we widen with all references in the expected type that are not subsumed by one of the parameter types. ## Changes: ### Refine rule for this widening We now widen the expected type of the right hand side of a class member as follows: Add all references of the declared type of this that are not subsumed by a capture set of a parameter type. ### Do expected type widening only in final classes Alex found a counter-example why this is required. See map5 in neg-customargs/captures/lazylists2.scala --- .../src/dotty/tools/dotc/cc/CaptureSet.scala | 9 ++- .../dotty/tools/dotc/transform/Recheck.scala | 7 +- .../tools/dotc/typer/CheckCaptures.scala | 16 +++++ .../neg-custom-args/captures/lazylists1.check | 7 ++ .../neg-custom-args/captures/lazylists1.scala | 27 ++++++++ .../neg-custom-args/captures/lazylists2.check | 45 +++++++++++++ .../neg-custom-args/captures/lazylists2.scala | 64 +++++++++++++++++++ .../captures/lazylists-mono.scala | 27 ++++++++ .../pos-custom-args/captures/lazylists.scala | 42 ++++++++++++ .../pos-custom-args/captures/lazylists1.scala | 35 ++++++++++ 10 files changed, 276 insertions(+), 3 deletions(-) create mode 100644 tests/neg-custom-args/captures/lazylists1.check create mode 100644 tests/neg-custom-args/captures/lazylists1.scala create mode 100644 tests/neg-custom-args/captures/lazylists2.check create mode 100644 tests/neg-custom-args/captures/lazylists2.scala create mode 100644 tests/pos-custom-args/captures/lazylists-mono.scala create mode 100644 tests/pos-custom-args/captures/lazylists.scala create mode 100644 tests/pos-custom-args/captures/lazylists1.scala diff --git a/compiler/src/dotty/tools/dotc/cc/CaptureSet.scala b/compiler/src/dotty/tools/dotc/cc/CaptureSet.scala index f8ca2f87e3c5..a26d0a952332 100644 --- a/compiler/src/dotty/tools/dotc/cc/CaptureSet.scala +++ b/compiler/src/dotty/tools/dotc/cc/CaptureSet.scala @@ -49,7 +49,14 @@ sealed abstract class CaptureSet extends Showable: /** Is this capture set definitely non-empty? */ final def isNotEmpty: Boolean = !elems.isEmpty - /** Cast to variable. @pre: @isConst */ + /** Cast to Const. @pre: isConst */ + def asConst: Const = this match + case c: Const => c + case v: Var => + assert(v.isConst) + Const(v.elems) + + /** Cast to variable. @pre: !isConst */ def asVar: Var = assert(!isConst) asInstanceOf[Var] diff --git a/compiler/src/dotty/tools/dotc/transform/Recheck.scala b/compiler/src/dotty/tools/dotc/transform/Recheck.scala index a61b736a9cc1..bbf60f5f4a79 100644 --- a/compiler/src/dotty/tools/dotc/transform/Recheck.scala +++ b/compiler/src/dotty/tools/dotc/transform/Recheck.scala @@ -196,12 +196,15 @@ abstract class Recheck extends Phase, IdentityDenotTransformer: bindType def recheckValDef(tree: ValDef, sym: Symbol)(using Context): Unit = - if !tree.rhs.isEmpty then recheck(tree.rhs, sym.info) + if !tree.rhs.isEmpty then recheckRHS(tree.rhs, sym.info, sym) def recheckDefDef(tree: DefDef, sym: Symbol)(using Context): Unit = val rhsCtx = linkConstructorParams(sym).withOwner(sym) if !tree.rhs.isEmpty && !sym.isInlineMethod && !sym.isEffectivelyErased then - inContext(rhsCtx) { recheck(tree.rhs, recheck(tree.tpt)) } + inContext(rhsCtx) { recheckRHS(tree.rhs, recheck(tree.tpt), sym) } + + def recheckRHS(tree: Tree, pt: Type, sym: Symbol)(using Context): Type = + recheck(tree, pt) def recheckTypeDef(tree: TypeDef, sym: Symbol)(using Context): Type = recheck(tree.rhs) diff --git a/compiler/src/dotty/tools/dotc/typer/CheckCaptures.scala b/compiler/src/dotty/tools/dotc/typer/CheckCaptures.scala index 69f184bd79ef..d815c09e99c9 100644 --- a/compiler/src/dotty/tools/dotc/typer/CheckCaptures.scala +++ b/compiler/src/dotty/tools/dotc/typer/CheckCaptures.scala @@ -322,6 +322,22 @@ class CheckCaptures extends Recheck: interpolateVarsIn(tree.tpt) curEnv = saved + override def recheckRHS(tree: Tree, pt: Type, sym: Symbol)(using Context): Type = + val pt1 = pt match + case CapturingType(core, refs, _) + if sym.owner.isClass && !sym.owner.isExtensibleClass + && refs.elems.contains(sym.owner.thisType) => + val paramCaptures = + sym.paramSymss.flatten.foldLeft(CaptureSet.empty) { (cs, p) => + val pcs = p.info.captureSet + (cs ++ (if pcs.isConst then pcs else CaptureSet.universal)).asConst + } + val declaredCaptures = sym.owner.asClass.givenSelfType.captureSet + pt.derivedCapturingType(core, refs ++ (declaredCaptures -- paramCaptures)) + case _ => + pt + recheck(tree, pt1) + override def recheckClassDef(tree: TypeDef, impl: Template, cls: ClassSymbol)(using Context): Type = for param <- cls.paramGetters do if param.is(Private) && !param.info.captureSet.isAlwaysEmpty then diff --git a/tests/neg-custom-args/captures/lazylists1.check b/tests/neg-custom-args/captures/lazylists1.check new file mode 100644 index 000000000000..29291c8044c0 --- /dev/null +++ b/tests/neg-custom-args/captures/lazylists1.check @@ -0,0 +1,7 @@ +-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazylists1.scala:25:63 ----------------------------------- +25 | def concat(other: {f} LazyList[A]): {this} LazyList[A] = ??? : ({xs, f} LazyList[A]) // error + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | Found: {xs, f} LazyList[A] + | Required: {Mapped.this, xs} LazyList[A] + +longer explanation available when compiling with `-explain` diff --git a/tests/neg-custom-args/captures/lazylists1.scala b/tests/neg-custom-args/captures/lazylists1.scala new file mode 100644 index 000000000000..02c7cb4ff3e5 --- /dev/null +++ b/tests/neg-custom-args/captures/lazylists1.scala @@ -0,0 +1,27 @@ +class CC +type Cap = {*} CC + +trait LazyList[+A]: + this: ({*} LazyList[A]) => + + def isEmpty: Boolean + def head: A + def tail: {this} LazyList[A] + +object LazyNil extends LazyList[Nothing]: + def isEmpty: Boolean = true + def head = ??? + def tail = ??? + +extension [A](xs: {*} LazyList[A]) + def map[B](f: {*} A => B): {xs, f} LazyList[B] = + final class Mapped extends LazyList[B]: + this: ({xs, f} Mapped) => + + def isEmpty = false + def head: B = f(xs.head) + def tail: {this} LazyList[B] = xs.tail.map(f) // OK + def drop(n: Int): {this} LazyList[B] = ??? : ({xs, f} LazyList[B]) // OK + def concat(other: {f} LazyList[A]): {this} LazyList[A] = ??? : ({xs, f} LazyList[A]) // error + new Mapped + diff --git a/tests/neg-custom-args/captures/lazylists2.check b/tests/neg-custom-args/captures/lazylists2.check new file mode 100644 index 000000000000..8e09dd26cccf --- /dev/null +++ b/tests/neg-custom-args/captures/lazylists2.check @@ -0,0 +1,45 @@ +-- [E163] Declaration Error: tests/neg-custom-args/captures/lazylists2.scala:50:10 ------------------------------------- +50 | def tail: {xs, f} LazyList[B] = xs.tail.map(f) // error + | ^ + | error overriding method tail in trait LazyList of type => {Mapped.this} LazyList[B]; + | method tail of type => {xs, f} LazyList[B] has incompatible type + +longer explanation available when compiling with `-explain` +-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazylists2.scala:18:4 ------------------------------------ +18 | final class Mapped extends LazyList[B]: // error + | ^ + | Found: {f, xs} LazyList[B] + | Required: {f} LazyList[B] +19 | this: ({xs, f} Mapped) => +20 | def isEmpty = false +21 | def head: B = f(xs.head) +22 | def tail: {this} LazyList[B] = xs.tail.map(f) +23 | new Mapped + +longer explanation available when compiling with `-explain` +-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazylists2.scala:27:4 ------------------------------------ +27 | final class Mapped extends LazyList[B]: // error + | ^ + | Found: {f, xs} LazyList[B] + | Required: {xs} LazyList[B] +28 | this: ({xs, f} Mapped) => +29 | def isEmpty = false +30 | def head: B = f(xs.head) +31 | def tail: {this} LazyList[B] = xs.tail.map(f) +32 | new Mapped + +longer explanation available when compiling with `-explain` +-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazylists2.scala:41:48 ----------------------------------- +41 | def tail: {this} LazyList[B] = xs.tail.map(f) // error + | ^^^^^^^^^^^^^^ + | Found: {f} LazyList[B] + | Required: {xs} LazyList[B] + +longer explanation available when compiling with `-explain` +-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/lazylists2.scala:59:48 ----------------------------------- +59 | def tail: {this} LazyList[B] = xs.tail.map(f) // error + | ^^^^^^^^^^^^^^ + | Found: {f} LazyList[B] + | Required: {Mapped.this} LazyList[B] + +longer explanation available when compiling with `-explain` diff --git a/tests/neg-custom-args/captures/lazylists2.scala b/tests/neg-custom-args/captures/lazylists2.scala new file mode 100644 index 000000000000..c31a1ae5d04f --- /dev/null +++ b/tests/neg-custom-args/captures/lazylists2.scala @@ -0,0 +1,64 @@ +class CC +type Cap = {*} CC + +trait LazyList[+A]: + this: ({*} LazyList[A]) => + + def isEmpty: Boolean + def head: A + def tail: {this} LazyList[A] + +object LazyNil extends LazyList[Nothing]: + def isEmpty: Boolean = true + def head = ??? + def tail = ??? + +extension [A](xs: {*} LazyList[A]) + def map[B](f: {*} A => B): {f} LazyList[B] = + final class Mapped extends LazyList[B]: // error + this: ({xs, f} Mapped) => + + def isEmpty = false + def head: B = f(xs.head) + def tail: {this} LazyList[B] = xs.tail.map(f) + new Mapped + + def map2[B](f: {*} A => B): {xs} LazyList[B] = + final class Mapped extends LazyList[B]: // error + this: ({xs, f} Mapped) => + + def isEmpty = false + def head: B = f(xs.head) + def tail: {this} LazyList[B] = xs.tail.map(f) + new Mapped + + def map3[B](f: {*} A => B): {xs} LazyList[B] = + final class Mapped extends LazyList[B]: + this: ({xs} Mapped) => + + def isEmpty = false + def head: B = f(xs.head) + def tail: {this} LazyList[B] = xs.tail.map(f) // error + new Mapped + + def map4[B](f: {*} A => B): {xs} LazyList[B] = + final class Mapped extends LazyList[B]: + this: ({xs, f} Mapped) => + + def isEmpty = false + def head: B = f(xs.head) + def tail: {xs, f} LazyList[B] = xs.tail.map(f) // error + new Mapped + + def map5[B](f: {*} A => B): LazyList[B] = + class Mapped extends LazyList[B]: + this: ({xs, f} Mapped) => + + def isEmpty = false + def head: B = f(xs.head) + def tail: {this} LazyList[B] = xs.tail.map(f) // error + class Mapped2 extends Mapped: + this: Mapped => + new Mapped2 + + diff --git a/tests/pos-custom-args/captures/lazylists-mono.scala b/tests/pos-custom-args/captures/lazylists-mono.scala new file mode 100644 index 000000000000..82c44abf703a --- /dev/null +++ b/tests/pos-custom-args/captures/lazylists-mono.scala @@ -0,0 +1,27 @@ +class CC +type Cap = {*} CC + +//------------------------------------------------- + +def test(E: Cap) = + + trait LazyList[+A]: + protected def contents: {E} () => (A, {E} LazyList[A]) + def isEmpty: Boolean + def head: A = contents()._1 + def tail: {E} LazyList[A] = contents()._2 + + class LazyCons[+A](override val contents: {E} () => (A, {E} LazyList[A])) + extends LazyList[A]: + def isEmpty: Boolean = false + + object LazyNil extends LazyList[Nothing]: + def contents: {E} () => (Nothing, LazyList[Nothing]) = ??? + def isEmpty: Boolean = true + + extension [A](xs: {E} LazyList[A]) + def map[B](f: {E} A => B): {E} LazyList[B] = + if xs.isEmpty then LazyNil + else + val cons = () => (f(xs.head), xs.tail.map(f)) + LazyCons(cons) diff --git a/tests/pos-custom-args/captures/lazylists.scala b/tests/pos-custom-args/captures/lazylists.scala new file mode 100644 index 000000000000..17d5f8546edc --- /dev/null +++ b/tests/pos-custom-args/captures/lazylists.scala @@ -0,0 +1,42 @@ +class CC +type Cap = {*} CC + +trait LazyList[+A]: + this: ({*} LazyList[A]) => + + def isEmpty: Boolean + def head: A + def tail: {this} LazyList[A] + +object LazyNil extends LazyList[Nothing]: + def isEmpty: Boolean = true + def head = ??? + def tail = ??? + +extension [A](xs: {*} LazyList[A]) + def map[B](f: {*} A => B): {xs, f} LazyList[B] = + final class Mapped extends LazyList[B]: + this: ({xs, f} Mapped) => + + def isEmpty = false + def head: B = f(xs.head) + def tail: {this} LazyList[B] = xs.tail.map(f) // OK + def concat(other: {f} LazyList[A]): {this, f} LazyList[A] = ??? : ({xs, f} LazyList[A]) // OK + if xs.isEmpty then LazyNil + else new Mapped + +def test(cap1: Cap, cap2: Cap) = + def f(x: String): String = if cap1 == cap1 then "" else "a" + def g(x: String): String = if cap2 == cap2 then "" else "a" + + val xs = + class Initial extends LazyList[String]: + this: ({cap1} Initial) => + + def isEmpty = false + def head = f("") + def tail = LazyNil + new Initial + val xsc: {cap1} LazyList[String] = xs + val ys = xs.map(g) + val ysc: {cap1, cap2} LazyList[String] = ys diff --git a/tests/pos-custom-args/captures/lazylists1.scala b/tests/pos-custom-args/captures/lazylists1.scala new file mode 100644 index 000000000000..4c8006fb0e29 --- /dev/null +++ b/tests/pos-custom-args/captures/lazylists1.scala @@ -0,0 +1,35 @@ +class CC +type Cap = {*} CC + +trait LazyList[+A]: + this: ({*} LazyList[A]) => + + def isEmpty: Boolean + def head: A + def tail: {this} LazyList[A] + +object LazyNil extends LazyList[Nothing]: + def isEmpty: Boolean = true + def head = ??? + def tail = ??? + +final class LazyCons[+T](val x: T, val xs: {*} () => {*} LazyList[T]) extends LazyList[T]: + this: ({*} LazyList[T]) => + + def isEmpty = false + def head = x + def tail: {this} LazyList[T] = xs() + +extension [A](xs: {*} LazyList[A]) + def map[B](f: {*} A => B): {xs, f} LazyList[B] = + if xs.isEmpty then LazyNil + else LazyCons(f(xs.head), () => xs.tail.map(f)) + +def test(cap1: Cap, cap2: Cap) = + def f(x: String): String = if cap1 == cap1 then "" else "a" + def g(x: String): String = if cap2 == cap2 then "" else "a" + + val xs = LazyCons("", () => if f("") == f("") then LazyNil else LazyNil) + val xsc: {cap1} LazyList[String] = xs + val ys = xs.map(g) + val ysc: {cap1, cap2} LazyList[String] = ys