NPE on exhaustive pattern matching switch expressions with sealed interface (Java 21, 3.19.500.v20240601-0610). #2521

agentgt · 2024-06-04T18:07:38Z

Given this code:

public sealed interface PatternMatching {
	
	record Stuff() implements PatternMatching {}
	
	static Stuff match(PatternMatching pm) {
		return switch(pm) {
		case Stuff s -> s;
		 // default -> ... you should not need as we exhausted it but Eclipse NPE w/o a default.
		}; 
	}
}

This happens on the last RC (3.19.500.v20240601-0610) and of course 4.31 and previous versions.

Caused by: java.lang.NullPointerException
    at java.base/java.util.Objects.requireNonNull(Objects.java:209)
    at java.base/java.util.Arrays$ArrayList.<init>(Arrays.java:4137)
    at java.base/java.util.Arrays.asList(Arrays.java:4122)
    at org.eclipse.jdt.internal.compiler.ast.SwitchStatement.getAllPermittedTypes(SwitchStatement.java:1381)
    at org.eclipse.jdt.internal.compiler.ast.SwitchStatement.checkAndFlagDefaultSealed(SwitchStatement.java:1368)
    at org.eclipse.jdt.internal.compiler.ast.SwitchStatement.resolve(SwitchStatement.java:1248)
    at org.eclipse.jdt.internal.compiler.ast.SwitchExpression.resolveType(SwitchExpression.java:418)
    at org.eclipse.jdt.internal.compiler.ast.ReturnStatement.resolve(ReturnStatement.java:362)
    at org.eclipse.jdt.internal.compiler.ast.Statement.resolveWithBindings(Statement.java:498)

EDIT Forgot to mention the work around. If you set the result of the switch to a variable then the error does not happen.

	static Stuff match(PatternMatching pm) {
		Stuff r = switch(pm) {
		case Stuff s -> s;
		}; 
		return r;
	}

However the above has issues with null analysis as it cannot infer that r is NonNull which I can file as a separate bug.

Thus I usually have to do:

	static Stuff match(PatternMatching pm) {
		Stuff r = switch(pm) {
		case Stuff s -> s;
		}; 
		return Objects.requireNonNull(r);
	}

The text was updated successfully, but these errors were encountered:

srikanth-sankaran · 2024-06-06T05:50:26Z

Compiling this code with 4.31 as well as with HEAD master I don't see any NPE:

public sealed interface PatternMatching {
	
	record Stuff() implements PatternMatching {}
	
	static Stuff match(PatternMatching pm) {
		return switch(pm) {
		case Stuff s -> s;
		 	// default -> ... you should not need as we exhausted it but Eclipse NPE w/o a default.
		}; 
	}
	
	public static void main(String[] args) {
		System.out.println(match(new Stuff()));
	}
}

The program compiles and when run prints Stuff[]

srikanth-sankaran · 2024-06-06T05:51:42Z

@agentgt - something appears amiss in the defect report - can you double check ??

mpalat · 2024-06-06T06:02:51Z

@agentgt maybe you can share the project as it is - just to make sure we have the same preferences as well in the tests

srikanth-sankaran · 2024-06-06T08:01:02Z

Guess what, while working on #2513 I do see this NPE. I will pursue that but it would still be useful to get a test case from here.

srikanth-sankaran · 2024-06-06T09:06:41Z

Guess what, while working on #2513 I do see this NPE. I will pursue that but it would still be useful to get a test case from here.

This is only sporadically reproduced in that project ... Don't have a reliable test case.

srikanth-sankaran · 2024-06-06T10:08:37Z

See also https://bugs.eclipse.org/bugs/show_bug.cgi?id=144976

srikanth-sankaran · 2024-06-06T10:23:07Z

This happens on the last RC (3.19.500.v20240601-0610) and of course 4.31 and previous versions.

Caused by: java.lang.NullPointerException
    at java.base/java.util.Objects.requireNonNull(Objects.java:209)
    at java.base/java.util.Arrays$ArrayList.<init>(Arrays.java:4137)
    at java.base/java.util.Arrays.asList(Arrays.java:4122)
    at org.eclipse.jdt.internal.compiler.ast.SwitchStatement.getAllPermittedTypes(SwitchStatement.java:1381)
    at org.eclipse.jdt.internal.compiler.ast.SwitchStatement.checkAndFlagDefaultSealed(SwitchStatement.java:1368)
    at org.eclipse.jdt.internal.compiler.ast.SwitchStatement.resolve(SwitchStatement.java:1248)
    at org.eclipse.jdt.internal.compiler.ast.SwitchExpression.resolveType(SwitchExpression.java:418)
    at org.eclipse.jdt.internal.compiler.ast.ReturnStatement.resolve(ReturnStatement.java:362)
    at org.eclipse.jdt.internal.compiler.ast.Statement.resolveWithBindings(Statement.java:498)

Do you have fuller logs ? Does this involve the HierarchyResolver ??

agentgt · 2024-06-06T13:30:40Z

@srikanth-sankaran I will attach more logs soon.

The project luckily is opensource: https://github.com/jstachio/rainbowgum

https://github.com/jstachio/rainbowgum/blob/e0af2870dcc5375d10515c31d72063d44a53dbc7/core/src/main/java/io/jstach/rainbowgum/LogAppender.java#L262

I change that bit of code to

		return switch (appender) {
		 case InternalLogAppender ia -> ia;
		 };

Roughly the same eclipse settings are here:
https://github.com/jstachio/rainbowgum/tree/main/etc/eea/prefs

I even have the Eclipse compiler set for analysis but is broken at the moment because I can't seem to get the eclipse annotation processor to behave properly on via the Maven plugin.

https://github.com/jstachio/rainbowgum/blob/main/bin/analyze.sh

Which you can run with bin/analyze.sh eclipse.

I think the setting that most likely might have an impact is I have Annotation based Null Analysis turned on.

agentgt · 2024-06-06T13:44:51Z

@srikanth-sankaran

The logs are at this public gist: https://gist.github.com/agentgt/232e1a9a6026a992f0053af83499db7e

It repeats several times.

I tried to put all the exceptions that are different instead of repeating them.

Do you have fuller logs ? Does this involve the HierarchyResolver ??

EDIT it appears the common entry point with all the calls is:

org.eclipse.jdt.internal.compiler.Compiler.resolve

and that appears to be called by lot more things than HieararchyResolver.

srikanth-sankaran · 2024-06-06T14:05:12Z

Thanks. I am on leave tomorrow. If I am able to locally observe the problem with the details you have outlined, I'll work on it early next week.

srikanth-sankaran · 2024-06-07T00:53:16Z

@agentgt - I have cloned that repo and imported the projects. LogAppender class seems to excluded from builds and indexing etc by default - What should I do ? If I say Open Type (Ctrl Shift T) LogAppender cannot be opened. I am able to do a textual search for LogAppender and locate the source file, but if I make some edits that doesn't seem to trigger a build,

agentgt · 2024-06-07T01:56:49Z

How are you importing the project?

I was using m2e as in the Maven pom should dictate everything with the exception of null analysis and things like formatting.

srikanth-sankaran · 2024-06-07T05:08:03Z

Well, I cloned they repo in a plain eclipse SDK installation and right clicked on repo and said import project from the working tree.

I am not familiar with maven. I'll take help from @jarthana and see if I am able to observe it locally.

agentgt · 2024-06-07T07:46:51Z

I will see if I can make a non maven project and check in the .project and .classpath later today if that helps.

srikanth-sankaran · 2024-06-07T10:55:28Z

I will see if I can make a non maven project and check in the .project and .classpath later today if that helps.

That would be very welcome. Thanks in advance.

jarthana · 2024-06-10T08:51:28Z

Well, I cloned they repo in a plain eclipse SDK installation and right clicked on repo and said import project from the working tree.

I am not familiar with maven. I'll take help from @jarthana and see if I am able to observe it locally.

I tried, but no luck reproducing this.

agentgt · 2024-06-10T14:48:49Z

I tried, but no luck reproducing this.

@srikanth-sankaran and @jarthana it requires turning on null analysis.

Preferences > Java Compiler > Error/Warnings > Enable annotation-based null analysis

I'm creating a project w/ just plain eclipse settings to show it.

jarthana · 2024-06-10T14:55:37Z

@srikanth-sankaran and @jarthana it requires turning on null analysis.

Preferences > Java Compiler > Error/Warnings > Enable annotation-based null analysis

I'm creating a project w/ just plain eclipse settings to show it.

Yes, I imported the projects, enabled null analysis and made the code change you mentioned. Anything else I am missing?

agentgt · 2024-06-10T15:05:26Z

Try this project:

https://github.com/agentgt/jdt-issues

This project is just a plain eclipse project. It has .classpath, .settings etc checked in.

Yes, I imported the projects, enabled null analysis and made the code change you mentioned. Anything else I am missing?

My guess is you may have not changed the code to make it break. The code I have checked in at rainbowgum purposely does not have it doing it.

the project I have included above does it.

agentgt · 2024-06-10T15:31:08Z

@jarthana

I updated the example eclipse project with a maven pom that will use the latest ECJ.

mvn clean verify -X

[DEBUG] Using JSR-199 EclipseCompiler
[DEBUG] ecj: using character set UTF-8
[DEBUG] ecj command line: [-noExit, -preserveAllLocals, -g:lines,vars,source, -source, 21, -target, 21, -encoding, UTF-8, -warn:+deprecation, -failOnWarning, -properties, /Users/agent/projects/jdt-issues/.settings/org.eclipse.jdt.core.prefs, -d, /Users/agent/projects/jdt-issues/target/classes, -s, /Users/agent/projects/jdt-issues/target/generated-sources/annotations, -classpath, /Users/agent/projects/jdt-issues/target/classes:/Users/agent/.m2/repository/org/eclipse/jdt/org.eclipse.jdt.annotation/2.2.700/org.eclipse.jdt.annotation-2.2.700.jar:/Users/agent/projects/jdt-issues/target/classes:]
[DEBUG] ecj input source files: [/Users/agent/projects/jdt-issues/src/main/java/bug/package-info.java, /Users/agent/projects/jdt-issues/src/main/java/bug/PatternMatching.java]
[DEBUG] ----------
1. ERROR in /Users/agent/projects/jdt-issues/src/main/java/bug/PatternMatching.java (at line 0)
	package bug;
	^
Internal compiler error: java.lang.NullPointerException at java.base/java.util.Objects.requireNonNull(Objects.java:233)
----------
java.lang.NullPointerException
	at java.base/java.util.Objects.requireNonNull(Objects.java:233)
	at java.base/java.util.Arrays$ArrayList.<init>(Arrays.java:4238)
	at java.base/java.util.Arrays.asList(Arrays.java:4223)
	at org.eclipse.jdt.internal.compiler.ast.SwitchStatement.getAllPermittedTypes(SwitchStatement.java:1381)
	at org.eclipse.jdt.internal.compiler.ast.SwitchStatement.checkAndFlagDefaultSealed(SwitchStatement.java:1368)
	at org.eclipse.jdt.internal.compiler.ast.SwitchStatement.resolve(SwitchStatement.java:1248)
	at org.eclipse.jdt.internal.compiler.ast.SwitchExpression.resolveType(SwitchExpression.java:418)
	at org.eclipse.jdt.internal.compiler.ast.ReturnStatement.resolve(ReturnStatement.java:362)
	at org.eclipse.jdt.internal.compiler.ast.Statement.resolveWithBindings(Statement.java:498)
	at org.eclipse.jdt.internal.compiler.ast.ASTNode.resolveStatements(ASTNode.java:726)
	at org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration.resolveStatements(AbstractMethodDeclaration.java:714)
	at org.eclipse.jdt.internal.compiler.ast.MethodDeclaration.resolveStatements(MethodDeclaration.java:409)
	at org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration.resolve(AbstractMethodDeclaration.java:612)
	at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.resolve(TypeDeclaration.java:1517)
	at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.resolve(TypeDeclaration.java:1646)
	at org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration.resolve(CompilationUnitDeclaration.java:666)
	at org.eclipse.jdt.internal.compiler.Compiler.process(Compiler.java:904)
	at org.eclipse.jdt.internal.compiler.ProcessTaskManager.run(ProcessTaskManager.java:145)
	at java.base/java.lang.Thread.run(Thread.java:1583)

So it is not a UI jdt bug if that helps.

EDIT the pom file is not meant for import (although it will probably work) but rather easier way to kick off ecj from command line.

jarthana · 2024-06-11T05:22:34Z

Try this project:

https://github.com/agentgt/jdt-issues

Reproduced. Will dig a bit more and see what's going on.

Yes, I imported the projects, enabled null analysis and made the code change you mentioned. Anything else I am missing?

My guess is you may have not changed the code to make it break. The code I have checked in at rainbowgum purposely does not have it doing it.

I am positive I made the code change. It looks like this:
return switch (appender) { case InternalLogAppender ia -> ia; }; //return Objects.requireNonNull((InternalLogAppender) appender); // TODO eclipse // bug.
But now we have a simply project, it doesn't matter.

srikanth-sankaran · 2024-06-11T08:42:29Z

Thanks @jarthana and @agentgt - I am investigating.

agentgt · 2024-06-11T12:38:16Z

I am positive I made the code change.

Apologies @jarthana ! I misread your previous comment that you had applied the change and thought you had just pulled it.

FWIW @srikanth-sankaran when I went code diving it looked like SourceTypeBinding permittedTypes is getting set to null at some point but that was based on a cursory glance on github.

agentgt · 2024-06-11T12:48:41Z

Also given this NPE only happens when null analysis is turned on (the irony) perhaps @stephan-herrmann might have some thoughts?

As a random side note perhaps the JDT code base should be annotated for null analysis? That might be something I could help with and would allow me to become more familiar with the JDT code base. Thoughts?

iloveeclipse · 2024-06-11T19:31:16Z

As a random side note perhaps the JDT code base should be annotated for null analysis? That might be something I could help with and would allow me to become more familiar with the JDT code base. Thoughts?

Would be great to have someone helping here. Not sure why Stephan didn't enabled that on JDT itself, probably due lot of legacy code that would require some effort. Or may be because of API compatibility? Better ask @stephan-herrmann about that.

stephan-herrmann · 2024-06-11T22:03:47Z

As a random side note perhaps the JDT code base should be annotated for null analysis? That might be something I could help with and would allow me to become more familiar with the JDT code base. Thoughts?

Would be great to have someone helping here. Not sure why Stephan didn't enabled that on JDT itself, probably due lot of legacy code that would require some effort. Or may be because of API compatibility? Better ask @stephan-herrmann about that.

:)

So far I simply didn't have the heart to start this, in part due to some properties of the fundamental design specifically of the compiler:

pervasive use of arrays, which have no effective means to ensure non-null elements
working in phases, like: after resolve() has completed, all AST nodes are assumed to have a non-null binding, but not before that
specific idioms like, e.g., loops over arrays, which hide the null check into another variable:

int len = arr != null ? arr.length : 0; 
if (len > 0) {
    for (int i = 0; i < len; i++) {
        arr[i] ... // compiler thinks arr can be null

Surely developing new code with null annotations is much, much easier than retrofitting this particular code base.

That said, it might still be feasible to do this in small increments. Perhaps one might actually start in a defensive way by only adding few @NonNull to method parameters and @Nullable to method returns, which reflect the fact of the matter where null parameters would immediately blow up, or were indeed null is returned. That would allow us to make existing contracts explicit, without changing any overall strategies. We'd have to see, whether a possible null return actually requires all clients to check for null - perhaps there are additional principles that guarantee non-null in specific situations.

We could try this, in small, careful steps.

stephan-herrmann · 2024-06-11T22:13:29Z

Also given this NPE only happens when null analysis is turned on (the irony)

Two possible reasons:

null analysis itself may not be null-safe
null analysis globally puts the compiler into annotation-aware mode (see classes TypeSystem vs. AnnotatableTypeSystem) and that mode may simply have other (more?) bugs then the mode that ignores annotations on type use.

srikanth-sankaran · 2024-06-12T05:16:17Z

Also given this NPE only happens when null analysis is turned on (the irony) perhaps @stephan-herrmann might have some thoughts?

That may be the case with this project but I certainly saw the same NPE in the context of #2513 where annotation based null analysis is not in the picture. I think there are some scenarios where a call of org.eclipse.jdt.internal.compiler.lookup.SourceTypeBinding.permittedTypes() happens ahead of a call to org.eclipse.jdt.internal.compiler.lookup.SourceTypeBinding.setPermittedTypes(ReferenceBinding[]) - OIOW, we are trying to touch permitted subtypes before they are computed.

I'll study this a bit more. I am hoping I can kill off more defects than just this one, such as #2108, #2093, #1998, #1808 - let us see how lucky I get :)

interface * Fixes eclipse-jdt#2521

srikanth-sankaran · 2024-06-12T07:19:56Z

@stephan-herrmann how does the assignment to org.eclipse.jdt.internal.compiler.lookup.SourceTypeBinding.superclass in org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.updateSupertypesWithAnnotations(Map<ReferenceBinding, ReferenceBinding>) tally with the advisory at the field declaration that reads // MUST NOT be modified directly, use setter ! ??

Not impacting this ticket ...

srikanth-sankaran · 2024-06-12T07:23:37Z

@stephan-herrmann how does the assignment to org.eclipse.jdt.internal.compiler.lookup.SourceTypeBinding.superclass in org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.updateSupertypesWithAnnotations(Map<ReferenceBinding, ReferenceBinding>) tally with the advisory at the field declaration that reads // MUST NOT be modified directly, use setter ! ??

Not impacting this ticket ...

Is it perhaps the case that we know fully well we are updating an annotated variant in that place and never the prototype - so no propagation is needed ??

srikanth-sankaran · 2024-06-12T07:34:32Z

I'll study this a bit more. I am hoping I can kill off more defects than just this one, such as #2108, #2093, #1998, #1808 - let us see how lucky I get :)

No such luck - this turned out to be a localized defect in annotation aware mode of the compiler @stephan-herrmann alluded to in his analysis.

…erface (eclipse-jdt#2553) * Maintain correlation between a prototype and its annotated variant * Fixes eclipse-jdt#2521

srikanth-sankaran · 2024-11-01T23:37:47Z

So far I simply didn't have the heart to start this, in part due to some properties of the fundamental design specifically of the compiler:

pervasive use of arrays, which have no effective means to ensure non-null elements

working in phases, like: after resolve() has completed, all AST nodes are assumed to have a non-null binding, but not before that

specific idioms like, e.g., loops over arrays, which hide the null check into another variable:

Does it make sense to add the the annotations projects as a required project so at least for documenting purposes we can add @nonnull etc to declarations ? Even if analysis itself is not triggered ?

agentgt mentioned this issue Jun 4, 2024

[Switch Expression][Null] Bogus null contract violation diagnostic at yield statements #2522

Closed

srikanth-sankaran self-assigned this Jun 5, 2024

srikanth-sankaran added a commit to srikanth-sankaran/eclipse.jdt.core that referenced this issue Jun 12, 2024

NPE on exhaustive pattern matching switch expressions with sealed

4e5c6aa

interface * Fixes eclipse-jdt#2521

This was referenced Jun 12, 2024

[sealed types] NPE on exhaustive pattern matching switch expressions with sealed interface #2553

Merged

[sealed types] ECJ should not accept type annotations on permitted types #2554

Closed

srikanth-sankaran closed this as completed in #2553 Jun 12, 2024

srikanth-sankaran closed this as completed in 2f57c19 Jun 12, 2024

srikanth-sankaran added this to the 4.33 M1 milestone Jun 12, 2024

stephan-herrmann mentioned this issue Jun 13, 2024

Suspicious direct assignment to SourceTypeBinding.superclass #2563

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NPE on exhaustive pattern matching switch expressions with sealed interface (Java 21, 3.19.500.v20240601-0610). #2521

NPE on exhaustive pattern matching switch expressions with sealed interface (Java 21, 3.19.500.v20240601-0610). #2521

agentgt commented Jun 4, 2024 •

edited

Loading

srikanth-sankaran commented Jun 6, 2024

srikanth-sankaran commented Jun 6, 2024

mpalat commented Jun 6, 2024

srikanth-sankaran commented Jun 6, 2024

srikanth-sankaran commented Jun 6, 2024

srikanth-sankaran commented Jun 6, 2024

srikanth-sankaran commented Jun 6, 2024

agentgt commented Jun 6, 2024 •

edited

Loading

agentgt commented Jun 6, 2024 •

edited

Loading

srikanth-sankaran commented Jun 6, 2024

srikanth-sankaran commented Jun 7, 2024 •

edited

Loading

agentgt commented Jun 7, 2024

srikanth-sankaran commented Jun 7, 2024

agentgt commented Jun 7, 2024

srikanth-sankaran commented Jun 7, 2024

jarthana commented Jun 10, 2024

agentgt commented Jun 10, 2024

jarthana commented Jun 10, 2024

agentgt commented Jun 10, 2024 •

edited

Loading

agentgt commented Jun 10, 2024 •

edited

Loading

jarthana commented Jun 11, 2024

srikanth-sankaran commented Jun 11, 2024

agentgt commented Jun 11, 2024

agentgt commented Jun 11, 2024

iloveeclipse commented Jun 11, 2024

stephan-herrmann commented Jun 11, 2024

stephan-herrmann commented Jun 11, 2024

srikanth-sankaran commented Jun 12, 2024

srikanth-sankaran commented Jun 12, 2024

srikanth-sankaran commented Jun 12, 2024

srikanth-sankaran commented Jun 12, 2024

srikanth-sankaran commented Nov 1, 2024

NPE on exhaustive pattern matching switch expressions with sealed interface (Java 21, 3.19.500.v20240601-0610). #2521

NPE on exhaustive pattern matching switch expressions with sealed interface (Java 21, 3.19.500.v20240601-0610). #2521

Comments

agentgt commented Jun 4, 2024 • edited Loading

srikanth-sankaran commented Jun 6, 2024

srikanth-sankaran commented Jun 6, 2024

mpalat commented Jun 6, 2024

srikanth-sankaran commented Jun 6, 2024

srikanth-sankaran commented Jun 6, 2024

srikanth-sankaran commented Jun 6, 2024

srikanth-sankaran commented Jun 6, 2024

agentgt commented Jun 6, 2024 • edited Loading

agentgt commented Jun 6, 2024 • edited Loading

srikanth-sankaran commented Jun 6, 2024

srikanth-sankaran commented Jun 7, 2024 • edited Loading

agentgt commented Jun 7, 2024

srikanth-sankaran commented Jun 7, 2024

agentgt commented Jun 7, 2024

srikanth-sankaran commented Jun 7, 2024

jarthana commented Jun 10, 2024

agentgt commented Jun 10, 2024

jarthana commented Jun 10, 2024

agentgt commented Jun 10, 2024 • edited Loading

agentgt commented Jun 10, 2024 • edited Loading

jarthana commented Jun 11, 2024

srikanth-sankaran commented Jun 11, 2024

agentgt commented Jun 11, 2024

agentgt commented Jun 11, 2024

iloveeclipse commented Jun 11, 2024

stephan-herrmann commented Jun 11, 2024

stephan-herrmann commented Jun 11, 2024

srikanth-sankaran commented Jun 12, 2024

srikanth-sankaran commented Jun 12, 2024

srikanth-sankaran commented Jun 12, 2024

srikanth-sankaran commented Jun 12, 2024

srikanth-sankaran commented Nov 1, 2024

agentgt commented Jun 4, 2024 •

edited

Loading

agentgt commented Jun 6, 2024 •

edited

Loading

agentgt commented Jun 6, 2024 •

edited

Loading

srikanth-sankaran commented Jun 7, 2024 •

edited

Loading

agentgt commented Jun 10, 2024 •

edited

Loading

agentgt commented Jun 10, 2024 •

edited

Loading