Skip to content

Backport "Don't warn for deprecated Thread.getId() calls in ChromeTrace" to 3.3 LTS#48

Closed
tgodzik wants to merge 1 commit intobackport-lts-3.3-19897from
backport-lts-3.3-21831
Closed

Backport "Don't warn for deprecated Thread.getId() calls in ChromeTrace" to 3.3 LTS#48
tgodzik wants to merge 1 commit intobackport-lts-3.3-19897from
backport-lts-3.3-21831

Conversation

@tgodzik
Copy link
Copy Markdown

@tgodzik tgodzik commented Feb 7, 2025

Backports scala#21831 to the 3.3.6.

PR submitted by the release tooling.
[skip ci]

@tgodzik tgodzik closed this Feb 13, 2025
@tgodzik tgodzik deleted the backport-lts-3.3-21831 branch February 13, 2025 18:41
tanishiking added a commit to tanishiking/scala3 that referenced this pull request Jan 30, 2026
Fixes scala#24201

When an object with
- A super class with a by-name parameter
- A by-name argument that gets lifted to a method
- Named arguments (a Block in the super call)

a `VerifyError` occurs at runtime because compiler generates a static method
that incorrectly tries to access `this` before initialization.

```scala
abstract class Foo[T](defaultValue: => T, arg1: Int = 1, arg2: Int = 2):
  def getValue: T = defaultValue

enum Baz:
  case E1, E2

object Baz extends Foo[Baz](Baz.E1, arg2 = 2)

@main def test = println(Baz.getValue)
```

The `Baz` in above Scala code, and it's compiled to the following JVM bytecode:

```
private Baz$();
  Code:
       // get default argument for `arg1` and store_1
       0: getstatic     scala#34    // Field Foo$.MODULE$:LFoo$;
       3: invokevirtual scala#38    // Method Foo$.$lessinit$greater$default$2:()I
       6: istore_1

       7: aload_0              // push uninitialized `this` for Foo.<init>

       // Trying to call this.defaultValue$1() but `this` is uninitialized!
       8: aload_0              // ← aload_0 on uninitialized this
       9: invokespecial scala#42    // Method defaultValue$1:()LBaz; ← INVALID!
      12: checkcast     scala#44    // class scala/Function0
      15: iload_1
      16: invokestatic  scala#48    // Method Baz$superArg$1:(Lscala/Function0;I)Lscala/Function0;
      19: iload_1
      20: iconst_2
      21: invokespecial scala#51    // Method Foo."<init>":(Lscala/Function0;II)V
      24: return
```

The JVM verifier rejects this because `aload_0` at offset 8 loads an
uninitialized `this` reference, and `invokespecial` at offset 9 tries
to call an instance method on it before the super constructor completes.

**Problem**

After Typer, the Baz constructor looks like this:

```scala
object Baz extends {
    def defaultValue$1: Baz = Baz.E1
    val arg1$1: Int = Foo.<init>$default$2[Baz]
    new Foo[Baz](defaultValue$1, arg1$1, arg2 = 2)
} { ... }
```

And, after `ElimByName` + `HoistSuperArgs`:
`HoistSuperArgs` creates a static helper method that evaluates complex super call arguments outside the constructor context.

```scala
object Baz extends {
    def defaultValue$1: Baz = Baz.E1   // still an instance method
    ...
    new Foo[Baz](Baz$$superArg$1()(defaultValue$1, arg1$1), ...)
                                   // ^ passes this.defaultValue$1 → VerifyError
} { ...
    private <static> def Baz$$superArg$1()(defaultValue$1: => Baz, ...): () ?=> Baz =
        () => defaultValue$1
}
```

  So it creates:
  private <static> def Baz$$superArg$1()(defaultValue$1: => Baz, arg1$1: Int): () ?=> Baz =
      () => defaultValue$1

  And the super call becomes:
  new Foo[Baz](Baz$$superArg$1()(defaultValue$1, arg1$1), arg1$1, 2)
                                 ^^^^^^^^^^^^^^^
                                 // This is a REFERENCE to the instance method!

The Tree
```scala
object Baz extends {
    def defaultValue$1: Baz = Baz.E1   // lifted by-name method (ExprType)
    val arg1$1: Int = Foo.<init>$default$2[Baz]
    new Foo[Baz](defaultValue$1, arg1$1, arg2 = 2)
} { ... }
```

```scala
object Baz extends {
    def defaultValue$1: Baz = Baz.E1   // still an instance method
    ...
    new Foo[Baz](Baz$$superArg$1()(defaultValue$1, arg1$1), ...)
                                   // ^ passes this.defaultValue$1 → VerifyError
} { ...
    private <static> def Baz$$superArg$1()(defaultValue$1: => Baz, ...): () ?=> Baz =
        () => defaultValue$1
}
```

However, to pass `defaultValue$1` as an argument to this static method, the compiler generates `this.defaultValue$1()`, which requires `this` to be initialized. So even though the static method itself doesn't use `this`, the call site access `this` before initializatin.

---

This commit fixes the invalid code generation by inlining `defaultValue$1`'s body directly into the static method, and now we don't need to reference `this` at the call site.

```scala
object Baz extends {
    // defaultValue$1 removed
    ...
    new Foo[Baz](Baz$$superArg$1()(arg1$1), ...)
} { ...
    private <static> def Baz$$superArg$1()(arg1$1: Int): () ?=> Baz =
        () => Baz.E1   // inlined body, no reference to this
}
```

In HoistSuperArgs, detect lifted by-name methods (synthetic methods with
ExprType info) in the super call Block, store their bodies, and inline
references to them in the hoisted static method. This avoids passing
instance method references to static methods during super call evaluation.
tanishiking added a commit to tanishiking/scala3 that referenced this pull request Jan 30, 2026
Fixes scala#24201

When an object with
- A super class with a by-name parameter
- A by-name argument that gets lifted to a method
- Named arguments (a Block in the super call)

a `VerifyError` occurs at runtime because compiler generates a static method
that incorrectly tries to access `this` before initialization.

```scala
abstract class Foo[T](defaultValue: => T, arg1: Int = 1, arg2: Int = 2):
  def getValue: T = defaultValue

enum Baz:
  case E1, E2

object Baz extends Foo[Baz](Baz.E1, arg2 = 2)

@main def test = println(Baz.getValue)
```

The `Baz` in above Scala code, and it's compiled to the following JVM bytecode:

```
private Baz$();
  Code:
       // get default argument for `arg1` and store_1
       0: getstatic     scala#34    // Field Foo$.MODULE$:LFoo$;
       3: invokevirtual scala#38    // Method Foo$.$lessinit$greater$default$2:()I
       6: istore_1

       7: aload_0              // push uninitialized `this` for Foo.<init>

       // Trying to call this.defaultValue$1() but `this` is uninitialized!
       8: aload_0              // ← aload_0 on uninitialized this
       9: invokespecial scala#42    // Method defaultValue$1:()LBaz; ← INVALID!
      12: checkcast     scala#44    // class scala/Function0
      15: iload_1
      16: invokestatic  scala#48    // Method Baz$superArg$1:(Lscala/Function0;I)Lscala/Function0;
      19: iload_1
      20: iconst_2
      21: invokespecial scala#51    // Method Foo."<init>":(Lscala/Function0;II)V
      24: return
```

The JVM verifier rejects this because `aload_0` at offset 8 loads an
uninitialized `this` reference, and `invokespecial` at offset 9 tries
to call an instance method on it before the super constructor completes.

**Problem**

After Typer, the Baz constructor looks like this:

```scala
object Baz extends {
    def defaultValue$1: Baz = Baz.E1
    val arg1$1: Int = Foo.<init>$default$2[Baz]
    new Foo[Baz](defaultValue$1, arg1$1, arg2 = 2)
} { ... }
```

And, after `ElimByName` + `HoistSuperArgs`:
`HoistSuperArgs` creates a static helper method that evaluates complex super call arguments outside the constructor context.

```scala
object Baz extends {
    def defaultValue$1: Baz = Baz.E1   // still an instance method
    ...
    new Foo[Baz](Baz$$superArg$1()(defaultValue$1, arg1$1), ...)
                                   // ^ passes this.defaultValue$1 → VerifyError
} { ...
    private <static> def Baz$$superArg$1()(defaultValue$1: => Baz, ...): () ?=> Baz =
        () => defaultValue$1
}
```

  So it creates:
  private <static> def Baz$$superArg$1()(defaultValue$1: => Baz, arg1$1: Int): () ?=> Baz =
      () => defaultValue$1

  And the super call becomes:
  new Foo[Baz](Baz$$superArg$1()(defaultValue$1, arg1$1), arg1$1, 2)
                                 ^^^^^^^^^^^^^^^
                                 // This is a REFERENCE to the instance method!

The Tree
```scala
object Baz extends {
    def defaultValue$1: Baz = Baz.E1   // lifted by-name method (ExprType)
    val arg1$1: Int = Foo.<init>$default$2[Baz]
    new Foo[Baz](defaultValue$1, arg1$1, arg2 = 2)
} { ... }
```

```scala
object Baz extends {
    def defaultValue$1: Baz = Baz.E1   // still an instance method
    ...
    new Foo[Baz](Baz$$superArg$1()(defaultValue$1, arg1$1), ...)
                                   // ^ passes this.defaultValue$1 → VerifyError
} { ...
    private <static> def Baz$$superArg$1()(defaultValue$1: => Baz, ...): () ?=> Baz =
        () => defaultValue$1
}
```

However, to pass `defaultValue$1` as an argument to this static method, the compiler generates `this.defaultValue$1()`, which requires `this` to be initialized. So even though the static method itself doesn't use `this`, the call site access `this` before initializatin.

---

This commit fixes the invalid code generation by inlining `defaultValue$1`'s body directly into the static method, and now we don't need to reference `this` at the call site.

```scala
object Baz extends {
    // defaultValue$1 removed
    ...
    new Foo[Baz](Baz$$superArg$1()(arg1$1), ...)
} { ...
    private <static> def Baz$$superArg$1()(arg1$1: Int): () ?=> Baz =
        () => Baz.E1   // inlined body, no reference to this
}
```

In HoistSuperArgs, detect lifted by-name methods (synthetic methods with
ExprType info) in the super call Block, store their bodies, and inline
references to them in the hoisted static method. This avoids passing
instance method references to static methods during super call evaluation.
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.

2 participants