Emit warnings for redundant capture references

Author: odersky

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

@@ -16,6 +16,9 @@ object CaptureOps:
     case Apply(_, Typed(SeqLiteral(elems, _), _) :: Nil) => elems
     case _ => Nil
 
+  extension (tree: Tree)
+    def toCaptureRef(using Context): CaptureRef = tree.tpe.asInstanceOf[CaptureRef]
+
   extension (cs: CaptureSet)
     def toAnnotation(using Context): Annotation =
       val refs = cs.elems.toList.map {
@@ -33,7 +36,7 @@ object CaptureOps:
   extension (annot: Annotation)
     def toCaptureSet(using Context): CaptureSet =
       assert(annot.symbol == defn.RetainsAnnot)
-      CaptureSet(retainedElems(annot.tree).map(_.tpe.asInstanceOf[CaptureRef])*)
+      CaptureSet(retainedElems(annot.tree).map(_.toCaptureRef)*)
         .showing(i"toCaptureSet $annot --> $result", capt)
 
   extension (tp: AnnotatedType)

compiler/src/dotty/tools/dotc/core/CaptureSet.scala

@@ -105,7 +105,8 @@ sealed abstract class CaptureSet extends Showable:
 
   /** The smallest superset (via <:<) of this capture set that also contains `ref`.
    */
-  def + (ref: CaptureRef)(using Context) = ++ (ref.singletonCaptureSet)
+  def + (ref: CaptureRef)(using Context): CaptureSet =
+    this ++ ref.singletonCaptureSet
 
   /** The largest capture set (via <:<) that is a subset of both `this` and `that`
    */
@@ -124,6 +125,9 @@ sealed abstract class CaptureSet extends Showable:
       case cs1: Const => Const(elems1)
       case cs1: Var => Diff(cs1, that)
 
+  def - (ref: CaptureRef)(using Context): CaptureSet =
+    this -- ref.singletonCaptureSet
+
   def filter(p: CaptureRef => Boolean)(using Context): CaptureSet = this match
     case cs1: Const => Const(elems.filter(p))
     case cs1: Var => Filtered(cs1, p)
@@ -160,7 +164,7 @@ object CaptureSet:
   private val emptySet = SimpleIdentitySet.empty
   @sharable private var varId = 0
 
-  val empty: CaptureSet = Const(emptySet)
+  val empty: CaptureSet.Const = Const(emptySet)
 
   /** The universal capture set `{*}` */
   def universal(using Context): CaptureSet =
@@ -169,7 +173,7 @@ object CaptureSet:
   /** Used as a recursion brake */
   @sharable private[core] val Pending = Const(SimpleIdentitySet.empty)
 
-  def apply(elems: CaptureRef*)(using Context): CaptureSet =
+  def apply(elems: CaptureRef*)(using Context): CaptureSet.Const =
     if elems.isEmpty then empty
     else Const(SimpleIdentitySet(elems.map(_.normalizedRef)*))
 
@@ -270,7 +274,7 @@ object CaptureSet:
     addSub(other)
 
   def mapRefs(xs: Refs, f: CaptureRef => CaptureSet)(using Context): CaptureSet =
-    (empty /: xs)((cs, x) => cs ++ f(x))
+    ((empty: CaptureSet) /: xs)((cs, x) => cs ++ f(x))
 
   type CompareResult = CompareResult.Type
 

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

@@ -2038,14 +2038,14 @@ object Types {
   trait CaptureRef extends SingletonType:
     private var myCaptureSet: CaptureSet = _
     private var myCaptureSetRunId: Int = NoRunId
-    private var mySingletonCaptureSet: CaptureSet = null
+    private var mySingletonCaptureSet: CaptureSet.Const = null
 
     def canBeTracked(using Context): Boolean
     final def isTracked(using Context): Boolean = canBeTracked && !captureSetOfInfo.isAlwaysEmpty
     def isRootCapability(using Context): Boolean = false
     def normalizedRef(using Context): CaptureRef = this
 
-    def singletonCaptureSet(using Context): CaptureSet =
+    def singletonCaptureSet(using Context): CaptureSet.Const =
       if mySingletonCaptureSet == null then
         mySingletonCaptureSet = CaptureSet(this.normalizedRef)
       mySingletonCaptureSet

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

@@ -60,8 +60,21 @@ object CheckCaptures:
       for ref <- tp.refs.elems do
         if (ref.captureSet frozen_<:< CaptureSet.empty) == CompareResult.OK then
           report.error(em"$ref cannot be tracked since its capture set is empty", pos)
+        else if tp.parent.captureSet.accountsFor(ref) then
+          report.warning(em"redundant capture: ${tp.parent} already accounts for $ref", pos)
     case _ =>
 
+  def checkWellformedPost(ann: Tree)(using Context): Unit =
+    def choices(prev: List[Tree], elems: List[Tree]): List[List[Tree]] = elems match
+      case Nil => Nil
+      case elem :: elems =>
+        List(elem :: (prev reverse_::: elems)) ++ choices(elem :: prev, elems)
+    for case first :: others <- choices(Nil, retainedElems(ann)) do
+      val firstRef = first.toCaptureRef
+      val remaining = CaptureSet(others.map(_.toCaptureRef)*)
+      if remaining.accountsFor(firstRef) then
+        report.warning(em"redundant capture: $remaining already accounts for $firstRef", ann.srcPos)
+
   private inline val disallowGlobal = true
 
 class CheckCaptures extends Recheck:
@@ -257,7 +270,11 @@ class CheckCaptures extends Recheck:
           case tree: TypeTree =>
             transformType(tree.tpe, inferred = false).foreachPart(
               checkWellformedPost(_, tree.srcPos))
-
+            tree.tpe.foreachPart {
+              case AnnotatedType(_, annot) =>
+                checkWellformedPost(annot.tree)
+              case _ =>
+            }
           case tree1 @ TypeApply(fn, args) if disallowGlobal =>
             for arg <- args do
               //println(i"checking $arg in $tree: ${arg.tpe.captureSet}")

compiler/test/dotty/tools/dotc/CompilationTests.scala

@@ -180,6 +180,7 @@ class CompilationTests {
       compileFile("tests/neg-custom-args/i7314.scala", defaultOptions.and("-Xfatal-warnings", "-source", "future")),
       compileFile("tests/neg-custom-args/feature-shadowing.scala", defaultOptions.and("-Xfatal-warnings", "-feature")),
       compileDir("tests/neg-custom-args/hidden-type-errors",  defaultOptions.and("-explain")),
+      compileFile("tests/neg-custom-args/capt-wf.scala", defaultOptions.and("-Ycc", "-Xfatal-warnings")),
     ).checkExpectedErrors()
   }
 

tests/neg-custom-args/captures/capt-wf.scala renamed to tests/neg-custom-args/capt-wf.scala

@@ -11,10 +11,12 @@ def test(c: Cap, other: String): Unit =
   val x3a: () => String = s1
   val s2 = () => if x1 == null then "" else "abc"
   val x4: {s2} C = ??? // OK
-  val x5: {c, c} C = ??? // warning: redundant
-  val x6: {c} {c} C = ??? // warning: redundant
-  val x7: {c} {*} C = ??? // warning: redundant
+  val x5: {c, c} C = ??? // error: redundant
+  val x6: {c} {c} C = ??? // error: redundant
+  val x7: {c} Cap = ??? // error: redundant
   val x8: {*} {c} C = ??? // OK
+  val x9: {c, *} C = ??? // error: redundant
+  val x10: {*, c} C = ??? // error: redundant
 
   def even(n: Int): Boolean = if n == 0 then true else odd(n - 1)
   def odd(n: Int): Boolean = if n == 1 then true else even(n - 1)