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

Unsound compilation of local classes in conjunction with exceptions #22051

Open
cpitclaudel opened this issue Nov 29, 2024 · 7 comments
Open
Labels
itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label

Comments

@cpitclaudel
Copy link

Compiler version

Scala CLI version: 1.5.4
Scala version (default): 3.5.2

Minimized code

def boundary[T](body: (T => RuntimeException) => T): T =
  case class Break(value: T) extends RuntimeException
  try body(Break.apply)
  catch case Break(t) => t

@main def main =
  boundary[Int]: EInt =>
    val v: String = boundary[String]: EString =>
      throw EInt(3)
    v.length

Output

Exception in thread "main" java.lang.ClassCastException: class java.lang.Integer cannot be cast to class java.lang.String (java.lang.Integer and java.lang.String are in module java.base of loader 'bootstrap')
	at crash$package$.main$$anonfun$1(crash.scala:8)
	at crash$package$.boundary(crash.scala:3)
	at crash$package$.main(crash.scala:7)
	at main.main(crash.scala:6)

Expectation

The program should either be rejected at compilation, or not crash. The problem seems to be that there's only one Break class at runtime, but the typechecker thinks that Break must contain a T.

@cpitclaudel cpitclaudel added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Nov 29, 2024
@dwijnand
Copy link
Member

What's EString and EInt?

@cpitclaudel
Copy link
Author

cpitclaudel commented Nov 29, 2024

They're just local names for the constructor of the exception type that's defined inside boundary. You could name them anything:

def boundary[T](body: (T => RuntimeException) => T): T =
  case class Break(value: T) extends RuntimeException
  try body(Break.apply)
  catch case Break(t) => t

@main def main =
  boundary[Int]: foo =>
    val v: String = boundary[String]: bar =>
      throw foo(3)
    v.length

@noti0na1
Copy link
Member

We can manually create a tag for each boundary execution:

def boundary[T](body: (T => RuntimeException) => T): T =
  val tag: AnyRef = new Object
  case class Break(value: T, tag: AnyRef) extends RuntimeException
  try body(Break.apply(_, tag))
  catch case Break(t, _: tag.type) => t

def test() =
  boundary[Int]: EInt =>
    val v: String = boundary[String]: EString =>
      throw EInt(3)
    v.length

test()

@bracevac
Copy link
Contributor

@noti0na1's solution is correct. The problem is erasure and how classes are handled. Break being defined in the body of method boundary does not mean there will be a unique Break class & instance per invocation of boundary. The class will be lifted out of boundary instead. It becomes clear when we inspect the program after one of the very late compiler phases, e.g., scalac -Xprint:repeatableAnnotations <filename>.scala.

So I am not sure whether there is anything to fix here.

@noti0na1
Copy link
Member

noti0na1 commented Nov 29, 2024

Well, I believe Java has the same behaviour, so I'm not sure whether we should "fix" this or not?

import java.util.function.Function;

public class BoundaryExample {
    public static <T> T boundary(Function<Function<T, RuntimeException>, T> body) {
        class Break extends RuntimeException {
            final T value;
            Break(T value) {
                this.value = value;
            }
        }
        try {
            return body.apply(value -> new Break(value));
        } catch (Break e) {
             return e.value;
        }
    }
    public static void main(String[] args) {
        Integer result = boundary(EInt -> {
            String v = boundary(EString -> {
                throw EInt.apply(3);
            });
            return v.length();
        });
        System.out.println(result);
    }
}
Exception in thread "main" java.lang.ClassCastException: class java.lang.Integer cannot be cast to class java.lang.String (java.lang.Integer and java.lang.String are in module java.base of loader 'bootstrap')
	at BoundaryExample.lambda$main$2(main.java:19)
	at BoundaryExample.boundary(main.java:12)
	at BoundaryExample.main(main.java:18) (exit status 1)

@sjrd
Copy link
Member

sjrd commented Nov 29, 2024

Gosh, if Java has the same issue, you might be able to get a paper out of it. 😯

That said in Scala we should try to emit a warning on the extractor, the same way we do for path-dependent classes for which we cannot statically enforce the right prefix.

@noti0na1
Copy link
Member

Some interesting find:

  • Kotlin and Swift disallows nested classes inside generic classes/functions that inherit from Throwable/Error to violate type-safety issue;
  • Go, python and js can produce correct behaviours (returning 3 at the end).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label
Projects
None yet
Development

No branches or pull requests

5 participants