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.

Sign up for GitHub

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

Jump to bottom

Crash in backend in secondary constructor edge case #18927

Closed
szymon-rd opened this issue Nov 14, 2023 · 4 comments · Fixed by #18949
Closed

Crash in backend in secondary constructor edge case #18927

szymon-rd opened this issue Nov 14, 2023 · 4 comments · Fixed by #18949
Assignees
@szymon-rd
Labels
area:backend itype:bug itype:crash

Comments

@szymon-rd
Copy link
Contributor

szymon-rd commented Nov 14, 2023 •
edited
Loading

Compiler version

3.4.0-RC1-bin-20231113-0dfe593-NIGHTLY

Minimized code

Minimized code from tests/init/neg/secondary-ctor4 test:

class A

class B {
  val a = new A

  class C(i: Int) {
    def this() = {
      this(1)
      class Inner() {
        println(a)
      }
    }
  }
}

Output

Error: Error while emitting /Users/srodziewicz/Projects/dotty/.scala-build/dotty_3200b05eac-ff28374a87/src_generated/main/xd.scala
assertion failed: Cannot emit primitive conversion from Lxd$_$B$C; to Lxd$_$B;

java.lang.AssertionError: assertion failed: Cannot emit primitive conversion from LB$C; to LB;
        at scala.runtime.Scala3RunTime$.assertFailed(Scala3RunTime.scala:8)
        at dotty.tools.backend.jvm.BCodeIdiomatic$JCodeMethodN.emitT2T(BCodeIdiomatic.scala:268)
        at dotty.tools.backend.jvm.BCodeBodyBuilder$PlainBodyBuilder.adapt(BCodeBodyBuilder.scala:1117)
        at dotty.tools.backend.jvm.BCodeBodyBuilder$PlainBodyBuilder.genAdaptAndSendToDest(BCodeBodyBuilder.scala:497)
        at dotty.tools.backend.jvm.BCodeBodyBuilder$PlainBodyBuilder.genLoadTo(BCodeBodyBuilder.scala:492)
        at dotty.tools.backend.jvm.BCodeBodyBuilder$PlainBodyBuilder.genLoad(BCodeBodyBuilder.scala:305)
        at dotty.tools.backend.jvm.BCodeBodyBuilder$PlainBodyBuilder.genStat(BCodeBodyBuilder.scala:111)
        at dotty.tools.backend.jvm.BCodeBodyBuilder$PlainBodyBuilder.genBlockTo$$anonfun$1(BCodeBodyBuilder.scala:1095)
        at scala.runtime.function.JProcedure1.apply(JProcedure1.java:15)
        at scala.runtime.function.JProcedure1.apply(JProcedure1.java:10)
        at scala.collection.immutable.List.foreach(List.scala:333)
        at dotty.tools.backend.jvm.BCodeBodyBuilder$PlainBodyBuilder.genBlockTo(BCodeBodyBuilder.scala:1095)
        at dotty.tools.backend.jvm.BCodeBodyBuilder$PlainBodyBuilder.genLoadTo(BCodeBodyBuilder.scala:459)
        at dotty.tools.backend.jvm.BCodeSkelBuilder$PlainSkelBuilder.emitNormalMethodBody$1(BCodeSkelBuilder.scala:893)
        at dotty.tools.backend.jvm.BCodeSkelBuilder$PlainSkelBuilder.genDefDef(BCodeSkelBuilder.scala:916)
  ...
@szymon-rd szymon-rd self-assigned this Nov 14, 2023
@szymon-rd szymon-rd changed the title Crash in secondary constructor edge case Crash in backend in secondary constructor edge case Nov 14, 2023
@szymon-rd
Copy link
Contributor Author

szymon-rd commented Nov 14, 2023 •
edited
Loading

Crashes in 3.2 and 3.1 as well, not a regression

@szymon-rd
Copy link
Contributor Author

szymon-rd commented Nov 14, 2023 •
edited
Loading

Tree checker also doesnt like this snippet:

Exception in thread "main" dotty.tools.dotc.reporting.UnhandledError: Reassignment to val $outer
        at dotty.tools.dotc.reporting.ThrowingReporter.doReport(ThrowingReporter.scala:14)
        at dotty.tools.dotc.reporting.Reporter.issueUnconfigured(Reporter.scala:159)
        at dotty.tools.dotc.reporting.Reporter.go$1(Reporter.scala:185)
        at dotty.tools.dotc.reporting.Reporter.issueIfNotSuppressed(Reporter.scala:204)
        at dotty.tools.dotc.reporting.Reporter.report(Reporter.scala:207)
        at dotty.tools.dotc.report$.error(report.scala:68)
        at dotty.tools.dotc.typer.Typer.reassignmentToVal$1(Typer.scala:1097)
        at dotty.tools.dotc.typer.Typer.typedAssign(Typer.scala:1171)
        at dotty.tools.dotc.typer.Typer.typedUnnamed$1(Typer.scala:3139)
        at dotty.tools.dotc.typer.Typer.typedUnadapted(Typer.scala:3216)
        at dotty.tools.dotc.typer.ReTyper.typedUnadapted(ReTyper.scala:174)
        at dotty.tools.dotc.transform.TreeChecker$Checker.typedUnadapted(TreeChecker.scala:398)
        at dotty.tools.dotc.typer.Typer.typed(Typer.scala:3293)
        at dotty.tools.dotc.typer.Typer.typed(Typer.scala:3297)
        at dotty.tools.dotc.transform.TreeChecker$Checker.typed(TreeChecker.scala:381)
        at dotty.tools.dotc.typer.Typer.traverse$1(Typer.scala:3346)
        at dotty.tools.dotc.typer.Typer.typedStats(Typer.scala:3365)
        at dotty.tools.dotc.transform.TreeChecker$Checker.typedStats(TreeChecker.scala:654)
        at dotty.tools.dotc.typer.Typer.typedBlockStats(Typer.scala:1182)
        at dotty.tools.dotc.typer.Typer.typedBlock(Typer.scala:1186)
        at dotty.tools.dotc.transform.TreeChecker$Checker.typedBlock$$anonfun$1$$anonfun$1(TreeChecker.scala:636)

@szymon-rd
Copy link
Contributor Author

szymon-rd commented Nov 15, 2023 •
edited
Loading

The cause

Here is a simplified version of the tree after constructors phase:

...
class C extends Object {
  ...
  def <init>($outer: B): Unit =
    {
      class Inner extends Object {
        def <init>($outer: B.this.C): Unit =
          {
            ...
            $outer = $outer
            ...
          }
        private val $outer: B.this.C
      }
      ()
    }
  private val $outer: B
}
...

The $outer = $outer comes from the fact that $outer is passed as the class arg to Inner. Constructors phase seems to be doing one thing wrong - it resolves the LHS to the $outer in C class, instead of Inner.

@szymon-rd
Copy link
Contributor Author

Found the bug:

mapOuter has this transformer defined:

   override def transform(tree: Tree)(using Context) = tree match {
        ....
        case tree: RefTree if tree.symbol.is(ParamAccessor) && tree.symbol.name == nme.OUTER =>
          ref(outerParam)
       ...

The problem is that if there are any nested constructors in the constructor, they will also be transformed. And all will point to the same $outer.

@szymon-rd szymon-rd mentioned this issue Nov 16, 2023
Merged
odersky added a commit that referenced this issue Nov 17, 2023
@odersky
Avoid too eager transform of $outer for lhs & accessor rhs (#18949)
3b974c5 
Fix #18927 

The transformer in `mapOuter` in `Constructors` was transforming trees
it should not:
```scala
   override def transform(tree: Tree)(using Context) = tree match {
        [....]
        case tree: RefTree if tree.symbol.is(ParamAccessor) && tree.symbol.name == nme.OUTER =>
          ref(outerParam)
       [...]
 ```
There were two problems:
 - This case transformed LHS of `$outer` assignments in constructors. So, instead of setting the `$outer` field in the current class with the constructor, it was replaced with the $outer of the outer class. That resulted in not assigning any value to the `$outer` in inner class, and double assignment to the val in outer class.
 - LHS of the accessor def was also transformed, so it was evaluated to the `$outer` of the outer class.

This only happened when the nested class is created in the secondary constructor, as the primary constructor is not transformed (only the template body)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@szymon-rd szymon-rd

Labels
area:backend itype:bug itype:crash
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Avoid too eager transform of $outer for lhs & accessor rhs scala/scala3
1 participant
@szymon-rd