Skip to content
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

Scala.js optimizer crashes when using js.dynamicImport #17344

Closed
lolgab opened this issue Apr 25, 2023 · 2 comments · Fixed by #17357
Closed

Scala.js optimizer crashes when using js.dynamicImport #17344

lolgab opened this issue Apr 25, 2023 · 2 comments · Fixed by #17357
Assignees
Milestone

Comments

@lolgab
Copy link
Contributor

lolgab commented Apr 25, 2023

Compiler version

3.3.1-RC1-bin-20230424-824295e-NIGHTLY

Minimized code

class Foo() {
  def foo() = 1
}
class Bar(foo: Foo) {
  def bar() = scalajs.js.dynamicImport(foo.foo())
}

object Main {
  def main(args: Array[String]): Unit = {
    val foo = new Foo()
    val bar = new Bar(foo)
    bar.bar()
  }
}

Output

Linker: Compute reachability: 13304 us
Linker: Assemble LinkedClasses: 716 us
Linker: 22690 us
Optimizer: Batch mode: true
Optimizer: Incremental part: 6120 us
Optimizer: Optimizing 203 methods.
1 targets failed
fullLinkJS org.scalajs.linker.frontend.optimizer.OptimizerCore$OptimizeException: The Scala.js optimizer crashed while optimizing static Bar$$anon$1.dynamicImport$;LBar;Ljava.lang.Object: java.util.NoSuchElementException: key not found: FieldID(ClassName<Bar$$anon$1>, FieldName<$outer>)
Methods attempted to inline:
* Bar$$anon$1.apply;Ljava.lang.Object

java.util.NoSuchElementException: key not found: FieldID(ClassName<Bar$$anon$1>, FieldName<$outer>)

Expectation

It links correctly when switching to Scala 2.13.10, so it should also using Scala 3

@lolgab lolgab added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 25, 2023
@sjrd sjrd self-assigned this Apr 25, 2023
@sjrd sjrd added area:scala.js and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 25, 2023
@sjrd
Copy link
Member

sjrd commented Apr 25, 2023

Thanks for the report. I can reproduce. Somehow that outer pointer, which is needed, disappears. It does not disappear if I "desugar" by hand the call to js.dynamicImport, so there must be something wrong in the phase that performs that desugaring.

@sjrd
Copy link
Member

sjrd commented Apr 26, 2023

Interestingly, explicitly writing Bar.this.foo.foo() instead of foo.foo() inside the dynamicImport makes it work.

sjrd added a commit to dotty-staging/dotty that referenced this issue Apr 26, 2023
…ts explicit.

Implicit references to the `this` of an outer class are made
explicit by the typer, and they need to be for `explicitOuter` to
do its job correctly.

When we desugar a `js.dynamicImport`, we move the code inside a
synthetic inner class. If it contains implicit references to an
enclosing class, we must make them explicit at that point.
@sjrd sjrd added this to the 3.3.1-RC1 milestone Apr 26, 2023
nicolasstucki added a commit that referenced this issue Apr 27, 2023
…plicit. (#17357)

Implicit references to the `this` of an outer class are made explicit by
the typer, and they need to be for `explicitOuter` to do its job
correctly.

When we desugar a `js.dynamicImport`, we move the code inside a
synthetic inner class. If it contains implicit references to an
enclosing class, we must make them explicit at that point.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants