Skip to content

Conversation

@som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Dec 29, 2025

The two usages of locally in the code base are actually just to parenthesize if-else in a boolean expression.

Because locally is not inline, that requires boxing the boolean, which is unused.

Inlining locally makes using it guilt-free. The signature is not changed here. To preserve bincompat, it overrides a non-inline method.

This commit fixes a doc typo and makes a boolean expression more readable with leading infix operators. Normally leading infix is requested during code review, but this was after a round of edits. Here, it makes a difference to help distinguish the if conditional from the then result expression.

Also fix the tests that fail on current JVM. It is locally annoying and a time waste at 666ff49.

@sjrd
Copy link
Member

sjrd commented Dec 29, 2025

Unfortunately, that is not backward binary compatible.

Fortunately, it shouldn't matter, because the JIT can eliminate it.

@som-snytt
Copy link
Contributor Author

som-snytt commented Dec 29, 2025

Ah yes. If I'm unable to add it to compiletime inlining, I'll change the usages to proper parens. Or, add it to dotc.util. Thanks @sjrd !

Edit: I noticed one weird trick --

@som-snytt som-snytt marked this pull request as ready for review December 30, 2025 06:04
@som-snytt som-snytt requested a review from a team as a code owner December 30, 2025 06:04
@odersky odersky requested a review from sjrd December 30, 2025 10:20
* @group utilities
*/
@inline def locally[T](@deprecatedName("x") x: T): T = x
inline override def locally[T](@deprecatedName("x") x: T): T = x
Copy link
Member

Choose a reason for hiding this comment

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

We had a chat at some point at EPFL about potentially having an annotation to require the method to remain in the binary. I would hold on doing such acrobatics until the final decision about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked at publicInBinary (which would be reasonable) or special-casing locally in Inlines, but this works today and is an insignificant delta.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cherry-picked @elidable(0) from another PR.

@som-snytt
Copy link
Contributor Author

I have already contributed "ignore line numbers in stack traces of tests" in another PR that is waiting patiently on the queue.

Failed to evaluate macro annotation '@crash'.
  Caused by class scala.NotImplementedError: an implementation is missing
    scala.Predef$.$qmark$qmark$qmark(Predef.scala:388)
    crash.transform(Macro_1.scala:7)

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.

3 participants