Skip to content

Undetected capture error with by-name parameters #21920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
odersky opened this issue Nov 10, 2024 · 2 comments · Fixed by #21863
Closed

Undetected capture error with by-name parameters #21920

odersky opened this issue Nov 10, 2024 · 2 comments · Fixed by #21863
Labels
area:experimental:cc Capture checking related area:reporting Error reporting including formatting, implicit suggestions, etc itype:bug
Milestone

Comments

@odersky
Copy link
Contributor

odersky commented Nov 10, 2024

Compiler version

3.6.1, PR

Minimized example

import language.experimental.captureChecking

trait Iterator[+A] extends IterableOnce[A]:
  self: Iterator[A]^ =>
  def next(): A

trait IterableOnce[+A] extends Any:
  def iterator: Iterator[A]^{this}

final class Cell[A](head: => IterableOnce[A]^):
  def headIterator: Iterator[A]^{this} = head.iterator

class File private ():
  private var closed = false

  def close() = closed = true

  def read() =
    assert(!closed, "File closed")
    1

object File:
  def open[T](f: File^ => T): T =
    val file = File()
    try
      f(file)
    finally
      file.close()

object Seq:
  def apply[A](xs: A*): IterableOnce[A] = ???

@main def Main() =
  val cell: Cell[File] = File.open(f => Cell(Seq(f)))
  val file = cell.headIterator.next()
  file.read()

Output

compiles

Expectation

Should give an error, as in the slightly changed version where we replace by-name with explicit functions:

import language.experimental.captureChecking

trait Iterator[+A] extends IterableOnce[A]:
  self: Iterator[A]^ =>
  def next(): A

trait IterableOnce[+A] extends Any:
  def iterator: Iterator[A]^{this}

final class Cell[A](head: () => IterableOnce[A]^):
  def headIterator: Iterator[A]^{this} = head().iterator

class File private ():
  private var closed = false

  def close() = closed = true

  def read() =
    assert(!closed, "File closed")
    1

object File:
  def open[T](f: File^ => T): T =
    val file = File()
    try
      f(file)
    finally
      file.close()

object Seq:
  def apply[A](xs: A*): IterableOnce[A] = ???

@main def Main() =
  val cell: Cell[File] = File.open(f => Cell(() => Seq(f)))
  val file = cell.headIterator.next()
  file.read()

This gives:

-- [E007] Type Mismatch Error: unsound-byname-2.scala:34:34 --------------------
34 |  val cell: Cell[File] = File.open(f => Cell(() => Seq(f)))
   |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |Found:    Cell[box File^{f, f²}]{val head: () ->? IterableOnce[box File^{f, f²}]^?}^?
   |Required: Cell[File]
   |
   |where:    f  is a reference to a value parameter
   |          f² is a reference to a value parameter
   |
   | longer explanation available when compiling with `-explain`
1 error found

I searched for a long time without getting to the root cause. What I did:

I compiled both programs with the first called unsound-byname.scala and the second called unsound-byname-2.scala with the following commands:

scc unsound-byname.scala -Ycc-log -color:never >& x
scc unsound-byname-2.scala -Ycc-log -color:never >& y

I then diffed the test output. There were several rabbit holes which were unproductive.

  • The second program expanded a PolyType [R] -> () -> R during setup while the first did not. It turned out this was because the second program uses an alias type while the first used the alias directly.
  • The target type of the by-name closure had different capture sets. I fixed that but it turned out it did not matter since the atrget type is an inferred type, so all capture sets are erased.

The one difference that remained was that the unsound program unsound-byname.scala produced several error messages of the form

existential match not found for ... 

where the other one did not. Something is inconsistent with existentials and that leads to errors being suppressed. I tried to dig deeper but could not find anything conclusive.

@odersky odersky added the stat:needs triage Every issue needs to have an "area" and "itype" label label Nov 10, 2024
@odersky
Copy link
Contributor Author

odersky commented Nov 10, 2024

By-name parameters get mapped in ElimByName to context functions. That's why the two program logs should be almost the same. And they are, except for this "existential match not found" business.

odersky added a commit to dotty-staging/dotty that referenced this issue Nov 11, 2024
A by-name Closure node, which is produced by phase ElimByName gets a target type to indicate
it's a contextual zero parameter closure. But for the purposes of rechecking and capture checking,
it needs to be treated like a function. In particular the type of the closure needs to be derived
from the result type of the anonymous function.

Fixes scala#21920
@odersky
Copy link
Contributor Author

odersky commented Nov 11, 2024

Fixed by #21863 with the commit above

odersky added a commit to dotty-staging/dotty that referenced this issue Nov 11, 2024
A by-name Closure node, which is produced by phase ElimByName gets a target type to indicate
it's a contextual zero parameter closure. But for the purposes of rechecking and capture checking,
it needs to be treated like a function. In particular the type of the closure needs to be derived
from the result type of the anonymous function.

Fixes scala#21920
@Gedochao Gedochao added itype:bug area:reporting Error reporting including formatting, implicit suggestions, etc area:experimental:cc Capture checking related and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Nov 12, 2024
@Gedochao Gedochao linked a pull request Nov 12, 2024 that will close this issue
odersky added a commit to dotty-staging/dotty that referenced this issue Nov 21, 2024
A by-name Closure node, which is produced by phase ElimByName gets a target type to indicate
it's a contextual zero parameter closure. But for the purposes of rechecking and capture checking,
it needs to be treated like a function. In particular the type of the closure needs to be derived
from the result type of the anonymous function.

Fixes scala#21920
@WojciechMazur WojciechMazur added this to the 3.6.3 milestone Nov 25, 2024
KacperFKorban pushed a commit to dotty-staging/dotty that referenced this issue Nov 29, 2024
A by-name Closure node, which is produced by phase ElimByName gets a target type to indicate
it's a contextual zero parameter closure. But for the purposes of rechecking and capture checking,
it needs to be treated like a function. In particular the type of the closure needs to be derived
from the result type of the anonymous function.

Fixes scala#21920
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:experimental:cc Capture checking related area:reporting Error reporting including formatting, implicit suggestions, etc itype:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants