Skip to content

Conversation

@odersky
Copy link
Contributor

@odersky odersky commented Oct 27, 2025

Don't perform the apply rule optimization under boxing

This was already the case for the select rule optimization in recheckSelection
but was missing for the corresponding rule for applications.

@odersky odersky requested a review from a team as a code owner October 27, 2025 14:33
@bracevac bracevac self-requested a review October 27, 2025 18:05
Copy link
Contributor

@bracevac bracevac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

else eagerHeadPrependIterator(it.next().iterator)(eagerHeadConcatIterators(it))
if !it.hasNext then Empty
else
caps.unsafe.unsafeDiscardUses:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That way, this escape hatch becomes unnecessary.

Copy link
Contributor Author

@odersky odersky Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I believe it's still good to have the escape hatch, in general.

@odersky
Copy link
Contributor Author

odersky commented Oct 28, 2025

This caused one regression in the standard library, here:

  private def eagerHeadConcatIterators[A](it: Iterator[collection.Iterable[A]^]^): LazyListIterable[A]^{it} =
    if !it.hasNext then Empty
    else
      eagerHeadPrependIterator
          (it.next().iterator)
          (eagerHeadConcatIterators(it))

Previously the access to it.next() was considered to have type it by applying the apply rule
incorrectly. It should be it*. This means we now have it* instead of it in the result type
and we also have an illegal use of it* leaking outside eagerHeadConcatIterators. The second
problem was fixed by adding an unsafe escape hatch unsafeDiscardUses that suppressed use recording.
This leads to:

  private def eagerHeadConcatIterators[A](it: Iterator[collection.Iterable[A]^]^): LazyListIterable[A]^{it*} =
    if !it.hasNext then Empty
    else
      eagerHeadPrependIterator
          (caps.unsafe.unsafeDiscardUses(it.next()).iterator)
          (eagerHeadConcatIterators(it))

This also did not compile since it claimed that the it @reachCapability was not a legal element of a capture set.
The root cause was that we forced some definitions already in parser, which can lead to confusion when compiling the
standard library itself. We now refrain from doing that and build the references to these annotations as untyped
trees all the way down.

@natsukagami
Copy link
Contributor

we also have an illegal use of it* leaking outside eagerHeadConcatIterators

Is this where we should annotate @use?

@odersky
Copy link
Contributor Author

odersky commented Oct 28, 2025

We don't have @use anymore.

This was already the case for the select rule optimization in recheckSelection
but was missing for the corresponding rule for applications.

This caused one regression in the standard library, here:
```
  private def eagerHeadConcatIterators[A](it: Iterator[collection.Iterable[A]^]^): LazyListIterable[A]^{it} =
    if !it.hasNext then Empty
    else
      eagerHeadPrependIterator
          (it.next().iterator)
          (eagerHeadConcatIterators(it))
```
Previously the access to `it.next()` was considered to have type `it` by applying the apply rule
incorrectly. It should be `it*`. This means we now have `it*` instead of `it` in the result type
and we also have an illegal use of `it*` leaking outside `eagerHeadConcatIterators`. The second
problem was fixed by adding an unsafe escape hatch `unsafeDiscardUses` that suppressed use recording.
This leads to:
```
  private def eagerHeadConcatIterators[A](it: Iterator[collection.Iterable[A]^]^): LazyListIterable[A]^{it*} =
    if !it.hasNext then Empty
    else
      eagerHeadPrependIterator
          (caps.unsafe.unsafeDiscardUses(it.next()).iterator)
          (eagerHeadConcatIterators(it))
```
This also did not compile since it claimed that the `it @reachCapability` was not a legal element of a capture set.
The root cause was that we forced some definitions already in parser, which can lead to confusion when compiling the
standard library itself. We now refrain from doing that and build the references to these annotations as untyped
trees all the way down.
@odersky odersky merged commit ec60015 into scala:main Oct 29, 2025
51 checks passed
@odersky odersky deleted the fix-apply-rule branch October 29, 2025 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants