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

Simple snippet crashing when Ycheck:all #18901

Closed
szymon-rd opened this issue Nov 10, 2023 · 2 comments · Fixed by #18924
Closed

Simple snippet crashing when Ycheck:all #18901

szymon-rd opened this issue Nov 10, 2023 · 2 comments · Fixed by #18924

Comments

@szymon-rd
Copy link
Contributor

szymon-rd commented Nov 10, 2023

Compiler version

3.4.0-RC1-bin-20231109-c7b3d7b-NIGHTLY

Minimized code

//> using options -Ycheck:all

trait B(val y: Int)

class C extends B(20)  {
  def foo(): Unit = println(y)
}

This code comes from tests/init/neg/trait1. More snippets like this are crashing. After implementing #18634 the pipeline goes on instead of failing on the first warn, and the compiler crashes.

Output (click arrow to expand)

Fatal compiler crash when compiling: tests/init/neg/trait1.scala:
assertion failed: bad type (C.this.y : (): Int) for C.this.y # -1
	at scala.runtime.Scala3RunTime$.assertFailed(Scala3RunTime.scala:8)
	at dotty.tools.dotc.transform.TreeChecker$Checker.typedIdent(TreeChecker.scala:435)
	at dotty.tools.dotc.typer.Typer.typedNamed$1(Typer.scala:3100)
	at dotty.tools.dotc.typer.Typer.typedUnadapted(Typer.scala:3215)
	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.typedExpr(Typer.scala:3409)
	at dotty.tools.dotc.typer.Applications.realApply$1(Applications.scala:958)
	at dotty.tools.dotc.typer.Applications.typedApply(Applications.scala:1118)
	at dotty.tools.dotc.typer.Applications.typedApply$(Applications.scala:352)
	at dotty.tools.dotc.transform.TreeChecker$Checker.typedApply(TreeChecker.scala:515)
	at dotty.tools.dotc.typer.Typer.typedUnnamed$1(Typer.scala:3132)
	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.ProtoTypes$FunProto.$anonfun$7(ProtoTypes.scala:509)
	at dotty.tools.dotc.typer.ProtoTypes$FunProto.cacheTypedArg(ProtoTypes.scala:432)
	at dotty.tools.dotc.typer.ProtoTypes$FunProto.typedArg(ProtoTypes.scala:510)
	at dotty.tools.dotc.typer.Applications$ApplyToUntyped.typedArg(Applications.scala:914)
	at dotty.tools.dotc.typer.Applications$ApplyToUntyped.typedArg(Applications.scala:914)
	at dotty.tools.dotc.typer.Applications$Application.addTyped$1(Applications.scala:606)
	at dotty.tools.dotc.typer.Applications$Application.matchArgs(Applications.scala:670)

@szymon-rd szymon-rd added itype:bug itype:crash stat:needs triage Every issue needs to have an "area" and "itype" label area:typer and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Nov 10, 2023
@szymon-rd
Copy link
Contributor Author

szymon-rd commented Nov 13, 2023

y Ident in line with def foo in class C stops being ParamAccessor after megaphase with elimErasedValueType for some reason, and needsSelect starts returning true (because due to the lack of ParamAccessor flag prefixIsElidable becomes false for the type of Ident y)

Edit: It changes in mixin phase, it drops the ParamAccessor

@szymon-rd
Copy link
Contributor Author

szymon-rd commented Nov 13, 2023

If I understand it correctly, prefixIsElidable assumes that the ParamAccessor flag is not dropped. But then, the logic in Mixin changes that. Is there any other way to know if the prefix is elidable after Mixin (in an efficient way)? @odersky

@szymon-rd szymon-rd self-assigned this Nov 13, 2023
sjrd added a commit that referenced this issue Nov 17, 2023
Fix #18901 

The check in `prefixIsElidable` was defined as follows:
```scala
tp.symbol.isParamOrAccessor && !pre.cls.is(Trait) && ctx.owner.enclosingClass == pre.cls
```

I assume that `!pre.cls.is(Trait)` condition was introduced to
accommodate for `Mixin` phase getting rid of `ParamAccessor` defined in
traits. However, the prefix does not indicate where the symbol is really
defined - it only represents the prefix from the perspective of the
current template, so it could be inherited. When it's inherited from a
trait, the prefix would be the current class, but the member still is
defined in the trait, and `Mixin` would get rid of the `ParamAccessor`
flag. Therefore, I changed this condition to the following:

```scala
tp.symbol.isParamOrAccesso && !pre.cls.is(Trait) && !tp.symbol.owner.is(Trait) && ctx.owner.enclosingClass == pre.cls
```
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.

1 participant