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

Record pattern matching with null gives wrong result #1804

Closed
datho7561 opened this issue Jan 2, 2024 · 6 comments · Fixed by #2016
Closed

Record pattern matching with null gives wrong result #1804

datho7561 opened this issue Jan 2, 2024 · 6 comments · Fixed by #2016
Assignees

Comments

@datho7561
Copy link
Contributor

Spotted by Jay in #1517 (comment)

Given the following code:

public class BooleanVariable {
	record Paper(int color) {}
	record Box<T>(T a) {}
	public static void main(String[] args) {
		Box<?> b = new Box<>(null);
		boolean res = b instanceof Box(Paper a);
		if (res) {
			System.out.println("res is true");
		} else {
			System.out.println("res is false");
		}
	}
}

ECJ prints "res is true" while javac prints "res is false".

@datho7561
Copy link
Contributor Author

datho7561 commented Jan 2, 2024

I'm planning on working on this

@datho7561
Copy link
Contributor Author

If you instead do:

boolean res = b instanceof Box(Paper a) && a == null;

It will print "res is false", since a completely different method is used to generate boolean methods when a part of an && expression. I'll try to repurpose that method so that it will work without &&.

@datho7561
Copy link
Contributor Author

datho7561 commented Jan 5, 2024

This example from Jay's comment also doesn't work:

public class X {
	record Paper(int color) {}
	record Box<T>(T a) {}
	public static void main(String argv[]) {
		foo(null, null);
	}
	public static void foo(String abc, String def) {
		Box<?> p = new Box<>(new Paper(0));
		boolean b = false;
		switch (p) {
			case Box(Paper a) -> {
				b = true;
				break;
			}
			default -> {
				b = false;
				break;
			}
		}
		System.out.println(b);
	}
}

However, it gives a VerifyError in the generated bytecode instead of the wrong result.

datho7561 added a commit to datho7561/eclipse.jdt.core that referenced this issue Jan 5, 2024
Boolean assignments are working, but switch statements aren't.
Fully removes unused pattern variables from stack map frame.

Fixes eclipse-jdt#1804

Signed-off-by: David Thompson <[email protected]>
datho7561 added a commit to datho7561/eclipse.jdt.core that referenced this issue Jan 5, 2024
Boolean assignments are working, but switch statements aren't.
Fully removes unused pattern variables from stack map frame.

Fixes eclipse-jdt#1804

Signed-off-by: David Thompson <[email protected]>
datho7561 added a commit to datho7561/eclipse.jdt.core that referenced this issue Jan 5, 2024
Boolean assignments are working, but switch statements aren't.
Fully removes unused pattern variables from stack map frame.

Fixes eclipse-jdt#1804

Signed-off-by: David Thompson <[email protected]>
datho7561 added a commit to datho7561/eclipse.jdt.core that referenced this issue Jan 9, 2024
Boolean assignments are working, but switch statements aren't.
Fully removes unused pattern variables from stack map frame.

Fixes eclipse-jdt#1804

Signed-off-by: David Thompson <[email protected]>
datho7561 added a commit to datho7561/eclipse.jdt.core that referenced this issue Jan 12, 2024
Boolean assignments are working, but switch statements aren't.
Fully removes unused pattern variables from stack map frame.

Fixes eclipse-jdt#1804

Signed-off-by: David Thompson <[email protected]>
datho7561 added a commit to datho7561/eclipse.jdt.core that referenced this issue Jan 24, 2024
datho7561 added a commit to datho7561/eclipse.jdt.core that referenced this issue Jan 24, 2024
Boolean assignments are working, but switch statements aren't.
Fully removes unused pattern variables from stack map frame.

Fixes eclipse-jdt#1804

Signed-off-by: David Thompson <[email protected]>
@srikanth-sankaran
Copy link
Contributor

@datho7561 - This is my plan for this ticket - I have assigned myself as a joint owner of this ticket. As mentioned in #1517 (comment) I am investigating and possibly undertaking a overhaul of code generation for patterns. If you don't mind, I would subsume this PR as a part of the larger work. Let us not rush this please. What do you say ?

@datho7561
Copy link
Contributor Author

Okay. Just as a heads up, this issue is a blocker for code generation in lists of patterns in case statements: #1742

I've figured out how to get code generation working in eclipse-jdtls#9, but I'll probably need to rewrite the code after your overhaul of pattern code generation.

datho7561 added a commit to datho7561/eclipse.jdt.core that referenced this issue Feb 6, 2024
datho7561 added a commit to datho7561/eclipse.jdt.core that referenced this issue Feb 6, 2024
@srikanth-sankaran
Copy link
Contributor

This example from Jay's comment also doesn't work:

public class X {
	record Paper(int color) {}
	record Box<T>(T a) {}
	public static void main(String argv[]) {
		foo(null, null);
	}
	public static void foo(String abc, String def) {
		Box<?> p = new Box<>(new Paper(0));
		boolean b = false;
		switch (p) {
			case Box(Paper a) -> {
				b = true;
				break;
			}
			default -> {
				b = false;
				break;
			}
		}
		System.out.println(b);
	}
}

However, it gives a VerifyError in the generated bytecode instead of the wrong result.

Happy to report that this one works fine on master, I'll push a regression test

srikanth-sankaran added a commit to srikanth-sankaran/eclipse.jdt.core that referenced this issue Feb 13, 2024
datho7561 added a commit to datho7561/eclipse.jdt.core that referenced this issue Feb 13, 2024
srikanth-sankaran pushed a commit to datho7561/eclipse.jdt.core that referenced this issue Feb 14, 2024
srikanth-sankaran added a commit that referenced this issue Feb 14, 2024
* Fix RecordPattern generation for standalone instanceof
* Fixes #1804
* Fixes #1985

Signed-off-by: David Thompson <[email protected]>
Co-authored-by: Srikanth Sankaran <[email protected]>
mickaelistria pushed a commit to mickaelistria/eclipse.jdt.core that referenced this issue Feb 16, 2024
mickaelistria pushed a commit to mickaelistria/eclipse.jdt.core that referenced this issue Feb 16, 2024
)

* Fix RecordPattern generation for standalone instanceof
* Fixes eclipse-jdt#1804
* Fixes eclipse-jdt#1985

Signed-off-by: David Thompson <[email protected]>
Co-authored-by: Srikanth Sankaran <[email protected]>
robstryker pushed a commit to robstryker/eclipse.jdt.core that referenced this issue Jul 18, 2024
robstryker pushed a commit to robstryker/eclipse.jdt.core that referenced this issue Jul 18, 2024
)

* Fix RecordPattern generation for standalone instanceof
* Fixes eclipse-jdt#1804
* Fixes eclipse-jdt#1985

Signed-off-by: David Thompson <[email protected]>
Co-authored-by: Srikanth Sankaran <[email protected]>
gayanper pushed a commit to gayanper/eclipse.jdt.core that referenced this issue Sep 7, 2024
gayanper pushed a commit to gayanper/eclipse.jdt.core that referenced this issue Sep 7, 2024
)

* Fix RecordPattern generation for standalone instanceof
* Fixes eclipse-jdt#1804
* Fixes eclipse-jdt#1985

Signed-off-by: David Thompson <[email protected]>
Co-authored-by: Srikanth Sankaran <[email protected]>
snjeza pushed a commit to snjeza/eclipse.jdt.core that referenced this issue Oct 21, 2024
)

* Fix RecordPattern generation for standalone instanceof
* Fixes eclipse-jdt#1804
* Fixes eclipse-jdt#1985

Signed-off-by: David Thompson <[email protected]>
Co-authored-by: Srikanth Sankaran <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants