Skip to content

Crash when refining a private type member #19929

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

Closed
EugeneFlesselle opened this issue Mar 12, 2024 · 6 comments · Fixed by #20188
Closed

Crash when refining a private type member #19929

EugeneFlesselle opened this issue Mar 12, 2024 · 6 comments · Fixed by #20188

Comments

@EugeneFlesselle
Copy link
Contributor

Compiler version

3.4.2-RC1

Minimized code

trait A:
  private type M

def foo(a: A{type M = Int}) =
  val _: a.M = ??? // crash

Note that replacing ??? with 0 gives an error instead.

Output (click arrow to expand)

  exception while retyping val _$1: a.M = ??? of class ValDef # -1

  An unhandled exception was thrown in the compiler.
  Please file a crash report here:
  https://github.com/scala/scala3/issues/new/choose
  For non-enriched exceptions, compile with -Yno-enrich-error-messages.

     while compiling: tests/playground/example.scala
        during phase: MegaPhase{elimErasedValueType, pureStats, vcElideAllocations, etaReduce, arrayApply, elimPolyFunction, tailrec, completeJavaEnums, mixin, lazyVals, memoize, nonLocalReturns, capturedVars}
                mode: Mode(ImplicitsEnabled)
     library version: version 2.13.12
    compiler version: version 3.4.2-RC1-bin-SNAPSHOT-nonbootstrapped-git-3694d95
            settings: -classpath /Users/eugeneflesselle/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.12/scala-library-2.13.12.jar:/Users/eugeneflesselle/Documents/dotty/library/../out/bootstrap/scala3-library-bootstrapped/scala-3.4.2-RC1-bin-SNAPSHOT-nonbootstrapped/scala3-library_3-3.4.2-RC1-bin-SNAPSHOT.jar -d /
cannot resolve reference to type (a : A{type M = Int}).M
the classfile defining the type might be missing from the classpath
@EugeneFlesselle EugeneFlesselle added itype:bug itype:crash stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 12, 2024
@Gedochao Gedochao added area:typer and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 12, 2024
odersky added a commit to dotty-staging/dotty that referenced this issue Mar 13, 2024
Fixes scala#19929

Two main changes:

 - In TypeErasure, throw a TypeError instead of a FatalError if a supertype of an applied
   type does not exist. That way, we get a proper error with a position.
 - Move some catch-and-rethrow logic from ReTyper to TreeChecker. ReTyper alreayd had special
   exceptions that disabled the logic for all uses of ReTyper except TreeChecker. Unfortunately
   the ReTyper override also disabled the special TypeError handling in Typer.
@odersky
Copy link
Contributor

odersky commented Apr 11, 2024

The fixing commit has been merged since. I think it was on another PR.

@EugeneFlesselle
Copy link
Contributor Author

The fixing commit has been merged since. I think it was on another PR.

The issue was fixed in #19983, but I do not think it included your commit. Should I just add a regression test or should those changes be merged ?

@EugeneFlesselle EugeneFlesselle self-assigned this Apr 12, 2024
odersky added a commit to dotty-staging/dotty that referenced this issue Apr 15, 2024
Fixes scala#19929

Two main changes:

 - In TypeErasure, throw a TypeError instead of a FatalError if a supertype of an applied
   type does not exist. That way, we get a proper error with a position.
 - Move some catch-and-rethrow logic from ReTyper to TreeChecker. ReTyper alreayd had special
   exceptions that disabled the logic for all uses of ReTyper except TreeChecker. Unfortunately
   the ReTyper override also disabled the special TypeError handling in Typer.
@EugeneFlesselle
Copy link
Contributor Author

EugeneFlesselle commented Apr 15, 2024

The test case no longer crashes and does not give an error either.

I expected it to mean we are refining the trait A with an unrelated public type member M, which would be ok, even if non-obvious. But that does not seem to be the case:

type Aux[m] = A {type M = m}
type Leak[a] = a match
  case Aux[m] => m

trait A:
  private type M
  def m: Leak[this.type]

def foo(a: A{type M = Int}): Int = a.m

compiles.

But should it ? I expected something like a non-private method refers to a private type in its signature error, or Leak[a] does not to conform to Int. @smarter

@smarter
Copy link
Member

smarter commented Apr 15, 2024

I expected it to mean we are refining the trait A with an unrelated public type member M, which would be ok, even if non-obvious.

Yes, that would be consistent with how a public def in a subclass can have the same name as a private def, otherwise adding a private definition could break backward compatibility.

@EugeneFlesselle
Copy link
Contributor Author

Yes, that would be consistent with how a public def in a subclass can have the same name as a private def, otherwise adding a private definition could break backward compatibility.

So it should not compile right ?

@smarter
Copy link
Member

smarter commented Apr 15, 2024

Right

EugeneFlesselle added a commit that referenced this issue Apr 15, 2024
Fixes #19929

Two main changes:

- In TypeErasure, throw a TypeError instead of a FatalError if a
supertype of an applied type does not exist. That way, we get a proper
error with a position.
- Move some catch-and-rethrow logic from ReTyper to TreeChecker. ReTyper
already had special exceptions that disabled the logic for all uses of
ReTyper except TreeChecker. Unfortunately the ReTyper override also
disabled the special TypeError handling in Typer.

The root cause of #19929 got fixed by another PR, but I think it's still
good to do the hardening of this commit.
@Kordyjan Kordyjan added this to the 3.5.0 milestone May 10, 2024
WojciechMazur pushed a commit that referenced this issue Jul 5, 2024
Fixes #19929

Two main changes:

 - In TypeErasure, throw a TypeError instead of a FatalError if a supertype of an applied
   type does not exist. That way, we get a proper error with a position.
 - Move some catch-and-rethrow logic from ReTyper to TreeChecker. ReTyper alreayd had special
   exceptions that disabled the logic for all uses of ReTyper except TreeChecker. Unfortunately
   the ReTyper override also disabled the special TypeError handling in Typer.

[Cherry-picked 822e792]
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.

5 participants