Discrepancy in null-coalescing operator

[ianharrigan]

https://try.haxe.org/#FCeC7D58

enum VariantType {
	VT_String(s:String);
	VT_Float(s:Float);
}

@:transitive
abstract Variant(VariantType) from VariantType {
	@:from static function fromString(s:String):Variant {
		return VT_String(s);
	}

	@:to public function toString():String {
		if (this == null) {
			return null;
		}
		return switch (this) {
			case VT_String(s): s;
			case VT_Float(s): Std.string(s);
		}
	}

	@:from static function fromFloat(s:Float):Variant {
		return VT_Float(s);
	}

	@:to public function toFloat():Null<Float> {
		if (this == null) {
			return null;
		}
		return switch (this) {
			case VT_Float(s): s;
			default: throw "Variant Type Error (" + this + ")";
		}
	}
}

class Test {
	static function main():Void {
		var variant1:Variant = 4.0;

		var testValue1:Float = variant1; // Works fine.
		// discrepency between "... != ... ? ... : ..." and "... ?? ... "
		var testValue2:Float = (variant1 != null) ? variant1 : 1.0; // Works fine.
		var testValue3:Float = variant1 ?? 1.0; // ERROR: Float should be Variant

		// some other oddities (unsure if related):
		// no type hint on testValue4
		var testValue4 = (variant1 != null) ? variant1 : 1.0; // ERROR: Float should be Variant
		// no type hint on testValue5
		var testValue5 = variant1 ?? 1.0; // ERROR: Float should be Variant
		// type hint testValue6 as Variant
		var testValue6:Variant = (variant1 != null) ? variant1 : 1.0; // Works fine.
		// type hint testValue7 as Variant
		var testValue7:Variant = variant1 ?? 1.0; // ERROR: Float should be Variant

		// using "cast" to get around compilation and see generated output:
		// though presumably cast is influencing the output also
		var testValue8:Float = (variant1 != null) ? variant1 : 1.0; // Works fine.
		// generated: variant1 != null ? Variant.toFloat(variant1) : 1.0
		var testValue9:Float = variant1 ?? cast 1.0; // Works fine.
		// generated: Variant.toFloat(variant1 != null ? variant1 : 1.0)
		var testValue10 = variant1 ?? cast 1.0; // Works fine.
		// generated: variant1 != null ? variant1 : 1.0
	}
}

Not sure if there is any more / better info than the source / comments?

[Simn]

Note that the != version also fails if there's no expected type: var a = (variant1 != null) ? variant1 : 1.0;. The problem is that the null coalescing operator always tries to find a minimal type, which doesn't work in this case. It shouldn't do that when there's already an expected type.

Edit: Just noticed that this case is hidden in all that mess already.

[ianharrigan]

Just noticed that this case is hidden in all that mess already.

Heh heh, sorry, yeah, i could / should have probably just kept the first 3 examples for brevity... :/

[Simn]

It's fine! I kinda hate these "look at all the possibly related scenarios" when investigating the issue, but it makes for a good test case.

[Simn]

Should work now! I've omitted cases 4 and 5 from your diary because that's a separate issue. It might be possible to support this by doing a normal left-to-right unification, but I'm not sure if it's worth the trouble.

@RblSb I'd appreciate if you could check if this change makes sense to you. I've extracted the with_type handling from make_if_then_else so that we consistently use the same mechanisms. I'm not very confident about iftype though and whether or not that one should already be passed to the new get_if_then_else_operands instead of with_type.

[ianharrigan]

oh thanks Simon... so fast! :)

I kinda hate these "look at all the possibly related scenarios"

Yeah, i think in reality the other scenarios might have just been noise - certainly some of them were at least

[Simn]

Well, this actually broke something on the C# target of all things:

ERROR  src/unit/TestNullCoalescing.hx:124: characters 4-15

 124 |    return null;
     |    ^^^^^^^^^^^
     | Null safety: Cannot return nullable value of Unknown<0> as Unknown<0>

 ERROR  src/unit/TestNullCoalescing.hx:126: characters 37-[41](https://github.com/HaxeFoundation/haxe/actions/runs/7192658073/job/19589592782#step:11:42)

 126 |   eq(item(1) ?? item(2) ?? item(3), null);
     |                                     ^^^^
     | Null safety: Cannot pass nullable value to not-nullable argument "v2" of function "eq".

[RblSb]

We have tests about Null unwrapping and 1 ?? 1.0 unification, so if things are green, we should be good. C# fails is interesting, something should be related to temp vars in generator or null<t> cleanups in it

[Simn]

I think it's because of a C#-specific hack for null-typing, so it's not a problem with my change here. The problem will disappear alongside the C# target.

[kLabz]

Note: this shouldn't be included in a bugfix release without #11706

[kLabz]

Unit test of 11425 fail at compile time for hl if I remove -D analyzer-optimize : | Don't know how to cast enum(unit.issues._Issue11425.VariantType) to null(f64)