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

?. Null-Safety Fails with match Method on Enums #11893

Open
NipponSunrise opened this issue Dec 18, 2024 · 9 comments
Open

?. Null-Safety Fails with match Method on Enums #11893

NipponSunrise opened this issue Dec 18, 2024 · 9 comments

Comments

@NipponSunrise
Copy link

Description of the Issue

The null-safety behavior of the ?. operator does not work correctly when calling the match method on a null value. This results in a TypeError: v is null, even though the ?. operator is expected to prevent method calls on null.


Minimal Reproducible Example (see here)

class Test {
	static function main() {
		var v:TestEnum = Value1;
		trace(v?.getName()); // output: Value1
		trace(v?.match(Value1)); // output: true

		v = Value2;
		trace(v?.getName()); // output: Value2
		trace(v?.match(Value1)); // output: false

		v = null;
		trace(v?.getName()); // output: 
		trace(v?.match(Value1)); // Null Pointer Exception occurs here
	}
}

enum TestEnum {
	Value1;
	Value2;
}

Expected Behavior

The ?. operator should properly handle null values and avoid calling the match method when the value is null. For example, if v = null, the statement trace(v?.match(Value1)); should evaluate to null without throwing an error.


Actual Behavior

When v?.match(Value1) is called and v is null, a TypeError: v is null is thrown, breaking the expected null-safety behavior of the ?. operator.


Environment

  • Haxe Version: 4.3.6
  • Target Platform: html5 (other not checked)

Additional Information

This issue violates the null-safety guarantees of the ?. operator, leading to unexpected runtime errors when relying on this functionality in code.

@Simn
Copy link
Member

Simn commented Dec 18, 2024

Since 46481f6 this gives an error.

I don't know if this is worth supporting. My intuition is that null?.match(pattern) looks like it should return false, not null, but that's not how ?. works in general.

@NipponSunrise
Copy link
Author

NipponSunrise commented Dec 18, 2024

Both v.match(Value1) and v?.match(Value1) result in a Null Pointer Exception, meaning the only current safe approach is the old-school if (v != null && v.match(Value1)). Regardless of how this issue is resolved, the ?. operator should behave correctly in all cases, as it is expected to ensure null-safety.

@Simn
Copy link
Member

Simn commented Dec 19, 2024

Yes that's why this errors now to communicate that null-safety isn't supported by ?.match.

I'd actually be open to making the normal value.match null-safe, i.e. make nullValue.match(null) work. Then there are no controversies about whether it should yield null or false.

@back2dos
Copy link
Member

back2dos commented Dec 19, 2024

FWIW nullable enums actually work fine in switch and with match: https://try.haxe.org/#F6eD114C

I'd actually be open to making the normal value.match null-safe, i.e. make nullValue.match(null) work. Then there are no controversies about whether it should yield null or false.

Well, dunno, given the above circumstances I would say:

  1. the null check could be generated if the type is non-nullable and nullsafety is disabled (so users would have a chance not to incur the code / speed cost)
  2. the ?.match is not supported error could suggest addjusting the type

@Simn
Copy link
Member

Simn commented Dec 19, 2024

Did you mean a Null<Option<String>> there? The switch version fails without it, the match version gets entirely optimized away by the analyzer.

@back2dos
Copy link
Member

Hmm, seems like the link I posted is wrong. So here's both Null<Option> and straight Option: https://try.haxe.org/#3C29d9a8 (the ifs are a bit awkward, because for the non-nullable case the compiler will simply turn !o.match(None | Some(_)) to false)

I would say that these behaviors really quite consistent. I wouldn't want the compiler to insert null checks when I say that the value is non-null. I could imagine that on some targets that might actually box the enum before doing the null check, which adds even more overhead than just the null check itself.

That being said, if the compiler supports.match correctly for nullable enums, could it not implement ?.match "simply" by promoting the type to Null<?> and then make a .match?

@Simn
Copy link
Member

Simn commented Dec 29, 2024

That being said, if the compiler supports.match correctly for nullable enums, could it not implement ?.match "simply" by promoting the type to Null<?> and then make a .match?

The problem isn't the implementation but the specification. It's unclear to me if nullValue?.match(pattern) should be false or null.

@back2dos
Copy link
Member

Welp, FWIW I think the current error is good enough ;)

I guess what I'm saying is that the following suggestion doesn't seem like a good idea, because of the hidden runtime overhead (and additional generated code):

I'd actually be open to making the normal value.match null-safe, i.e. make nullValue.match(null) work. Then there are no controversies about whether it should yield null or false.


The problem isn't the implementation but the specification. It's unclear to me if nullValue?.match(pattern) should be false or null.

Still, let's say we would want to support it and for the sake of consistency, it would be null. What problems might that produce?

@Simn
Copy link
Member

Simn commented Dec 29, 2024

To me there's a conflict between consistency and usefulness here because I'd consider a null-safe match that returns true/false more useful than a version that gives null and is typed as Null<Bool>.

Then again it might not make much difference in any actual logic assuming that null and false behave the same way with regards to boolean logic. It seems like a pointless tri-state, but I guess that's true for Null<Bool> in general.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants