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

Unsoundness due to erased #4060

Closed
hobwekiva opened this issue Mar 1, 2018 · 11 comments · Fixed by #16111
Closed

Unsoundness due to erased #4060

hobwekiva opened this issue Mar 1, 2018 · 11 comments · Fixed by #16111

Comments

@hobwekiva
Copy link

hobwekiva commented Mar 1, 2018

Related to #50 #4042 #4031.

object App {
  def coerce[U, V](u: U): V = {
    trait X { type R >: U }
    trait Y { type R = V }

    class T[A <: X](ghost val a: A)(val value: a.R)

    object O { lazy val x : Y & X = ??? }

    val a = new T[Y & X](O.x)(u)
    a.value
  }

  def main(args: Array[String]): Unit = {
    val x: Int = coerce[String, Int]("a")
    println(x + 1)
  }
}

//////////////////////////////////////////////////////////////

Exception in thread "main" java.lang.ClassCastException: java.lang.String cannot be cast to java.lang.Integer
	at scala.runtime.BoxesRunTime.unboxToInt(BoxesRunTime.java:101)
	at App$.main(HelloWorld.scala:15)
	at App.main(HelloWorld.scala)
@Blaisorblade
Copy link
Contributor

Blaisorblade commented Mar 1, 2018

In a hurry, but since #4031 is open, is ghost needed to get the unsoundness here? In fact, I suspect ghost should mask this bug. I’d expect two errors:

  • a lazy val is not stable, so can’t instantiate params with mentioned in path-dependent types (WIP in Fix #4031: Check arguments of dependent methods for realizability #4036);
  • a lazy val is not stable, so it can’t instantiate ghost arguments — if they’re supposed to be used as proofs, the language of unused arguments need to be pure and strongly normalizing. None of this is mentioned by the documentation for ghost, but it seems to be a loophole in the implied guarantees.

@nicolasstucki @odersky what do you think? We should probably have a chat about this.

@hobwekiva
Copy link
Author

hobwekiva commented Mar 1, 2018

ghost is necessary because otherwise O.x will get evaluated. You can pass ??? directly to ghost instead with the same result:

object App {
  def coerce[U, V](u: U): V = {
    trait X { type R >: U }
    trait Y { type R = V }

    class T[A <: X](ghost val a: A)(val value: a.R)

    val a = new T[Y & X]( ??? : Y & X )(u)
    a.value
  }

  def main(args: Array[String]): Unit = {
    val x: Int = coerce[String, Int]("a")
    println(x + 1)
  }
}

@hobwekiva hobwekiva changed the title Unsoundness due to lazy + unused. Unsoundness due to unused. Mar 2, 2018
@Blaisorblade
Copy link
Contributor

Blaisorblade commented Mar 2, 2018

OK. I suppose already isStable in Typer could recognize ghost definitions as unstable, since figuring this out requires no forcing.

@nicolasstucki nicolasstucki changed the title Unsoundness due to unused. Unsoundness due to unused Mar 2, 2018
@Blaisorblade Blaisorblade changed the title Unsoundness due to unused Unsoundness due to ghost Mar 2, 2018
@Blaisorblade
Copy link
Contributor

@alexknvl I renamed unused to ghost throughout the issue because of #4061.

@Blaisorblade
Copy link
Contributor

Thinking again:

  • currently ghost only promises that values are erased, so we should mark everything ghost as unstable;
  • if someday we finally get a good effect analysis, we might use it to check statically that ghost constructs are strongly normalizing (which we'd want for proofs as I suggested), but that's a major improvement. When that's done, it might be possible to allow treating ghost values as stable, but I'd want to think it through first (if not sketch an extension of the soundness proofs). Significantly, we barely know a Scala subset which is strongly normalizing (status in http://drops.dagstuhl.de/opus/volltexte/2017/7276/).

@Blaisorblade
Copy link
Contributor

Implemented it through isStable in SymDenotations, not Typer, since it was easier to access the flag there (as IIUC it's a flag on symbols, not types themselves).

Blaisorblade added a commit to dotty-staging/dotty that referenced this issue Mar 4, 2018
Blaisorblade added a commit to dotty-staging/dotty that referenced this issue Mar 7, 2018
Blaisorblade added a commit to dotty-staging/dotty that referenced this issue Mar 8, 2018
nicolasstucki added a commit that referenced this issue Mar 9, 2018
Fix #4060 by marking ghost symbols as unstable
@odersky
Copy link
Contributor

odersky commented Jul 4, 2018

Unfortunately, this makes erased vals practically useless. We should backout this fix and mark erased vals as unrealizable instead. erased is really the same as lazy in this respect. Either implies that the right-hand side will be constructed.

@Blaisorblade
Copy link
Contributor

Blaisorblade commented Dec 10, 2018

Reopening given the resolution to #4031 in #5568. When applying a dependent method/function to a dependent argument a, we realized that we need not check a's realizability because a will be a value; but we must if a is erased. Concretely, we get a CCE from:

object App {
  trait A { type L >: Any}
  //def upcast(a: A, x: Any): a.L = x
  def upcast(erased a: A)(x: Any): a.L = x
  //lazy val p: A { type L <: Nothing } = p
  erased val p: A { type L <: Nothing } = p
  def coerce(x: Any): Int = upcast(p)(x)

  def main(args: Array[String]): Unit = {
    println(coerce("Uh oh!"))
  }
}

@Blaisorblade
Copy link
Contributor

Martin today suggested we might want to just require all erased arguments to be realizable, to make this simpler to implement.
But there are open questions on erased itself; until they are resolved, this should be put on hold.

@nicolasstucki nicolasstucki changed the title Unsoundness due to ghost Unsoundness due to erased May 3, 2019
@Blaisorblade Blaisorblade removed their assignment Jul 1, 2020
@bishabosha
Copy link
Member

most recent example from @Blaisorblade does still compile and lead to a class cast exception

@odersky
Copy link
Contributor

odersky commented Sep 26, 2022

I think we should check that all erased definitions are realizable. See CheckRealizability.scala. Spec needs to be updated as well.

@odersky odersky removed their assignment Sep 26, 2022
@natsukagami natsukagami self-assigned this Sep 26, 2022
natsukagami added a commit to natsukagami/dotty that referenced this issue Sep 27, 2022
For i11896, we now no longer allow an erased val to be of a
top-level type declaration X (since it is not a concrete type,
per realizability rules). `class X` is used instead.

Another test, `top-level-type`, was added to check this behaviour.
natsukagami added a commit to natsukagami/dotty that referenced this issue Nov 8, 2022
For i11896, we now no longer allow an erased val to be of a
top-level type declaration X (since it is not a concrete type,
per realizability rules). `class X` is used instead.

Another test, `top-level-type`, was added to check this behaviour.

Fix some tests using top-level types for erasedValue

Add parameter-only dependency case
natsukagami added a commit to natsukagami/dotty that referenced this issue Nov 8, 2022
natsukagami added a commit that referenced this issue Nov 8, 2022
Fix #4060.

Per @odersky's [comment](#4060 (comment)),
we wish to add realizability checks to all erased vals.

- [x] Add realization check to erased definitions typer
- [x] Add note about realizibility in the spec
- [x] Add tests for #4060
little-inferno pushed a commit to little-inferno/dotty that referenced this issue Jan 25, 2023
@Kordyjan Kordyjan added this to the 3.3.0 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment