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

[Switch][Sealed types] Bad static analysis with the old switch syntax + an exhautive pattern matching on a sealed type throws a MatchException #3096

Closed
forax opened this issue Oct 16, 2024 · 5 comments · Fixed by #3098
Assignees
Labels
bug Something isn't working compiler Eclipse Java Compiler (ecj) related issues
Milestone

Comments

@forax
Copy link

forax commented Oct 16, 2024

Hello,
One of my TA has found the following bug, if a switch is exhautive, i.e. the cases cover all subtypes of the sealed interface, as the JLS says the compiler adds a default -> throw new MatchException(); and a break is added in the last case if necessary.

Here Eclipse fails to properly analyze the if (bs == null) so it does not insert the break after the last case so at runtime, the case B fallthrough and throws a MatchException.

public class EclipseBugFallThroughSwitch {
  sealed interface I permits A, B {}
  record A(String s) implements I {}
  record B(String s) implements I {}
  
  public void add(I i) {
    switch (i) {
    case A a:
      break;
    case B b:
      if (b.s == null) {
        throw new NullPointerException();
      }
      //break;  // this fix the issue
    }
  }
  
  public static void main(String[] args) {
    var container = new EclipseBugFallThroughSwitch();
    container.add(new B("bar"));
    
    // Exception in thread "main" java.lang.MatchException
    // at EclipseBugFallThroughSwitch.add(EclipseBugFallThroughSwitch.java:9)
    // at EclipseBugFallThroughSwitch.main(EclipseBugFallThroughSwitch.java:25)
  }
}
@srikanth-sankaran srikanth-sankaran self-assigned this Oct 16, 2024
@srikanth-sankaran
Copy link
Contributor

Thanks for the report Rémi. Reproduced.

Smaller test case:

public sealed interface X permits X.R {
  record R(String s) implements X {}
  
  public static void add(X x) {
    switch (x) {
    case R r:
      if (r.s == null) {
        throw new NullPointerException();
      }
    }
  }
  
  public static void main(String[] args) {
    add(new R("bar"));
  }
}

@srikanth-sankaran srikanth-sankaran changed the title [compiler] Bad static analysis with the old switch syntax + an exhautive pattern matching on a sealed type throws a MatchException [Switch][Sealed types] Bad static analysis with the old switch syntax + an exhautive pattern matching on a sealed type throws a MatchException Oct 16, 2024
@srikanth-sankaran srikanth-sankaran added compiler Eclipse Java Compiler (ecj) related issues bug Something isn't working labels Oct 16, 2024
@srikanth-sankaran
Copy link
Contributor

Problem dates back to 4.30

srikanth-sankaran added a commit to srikanth-sankaran/eclipse.jdt.core that referenced this issue Oct 16, 2024
an exhautive pattern matching on a sealed type throws a MatchException

* Fixes eclipse-jdt#3096
@srikanth-sankaran srikanth-sankaran added this to the 4.34 M2 milestone Oct 16, 2024
@srikanth-sankaran
Copy link
Contributor

Same underlying problem as #2513 - the proposed fix there was using too much local context. That deficiency should be addressed by the current PR #3098

@srikanth-sankaran
Copy link
Contributor

To check and to address separately if needed: whether JDK18- code falls into the compiler inserted default that throws an IncompatibleClassChangeError

@srikanth-sankaran
Copy link
Contributor

To check and to address separately if needed: whether JDK18- code falls into the compiler inserted default that throws an IncompatibleClassChangeError

Yes, this scenario is correctly handled. I have added more tests here: #3099

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler Eclipse Java Compiler (ecj) related issues
Projects
None yet
2 participants