Null coalescing operator

[RblSb]

Need to know if current implementation is fine.
final a = null ?? 2; shows null safety error, same as with a = null == null ? 2 : null expression.

Closes #10478

[Simn]

Check this out:

var count = 0;

function call() {
	count++;
	return "1";
}

function main() {
	var a = call() ?? "default";
	trace(count); // oh oh
}

[Simn]

I've made a fix. Didn't really mean to push that to your remote but oh well.

[RblSb]

I'm ok with any changes in my branches, thank you

[RblSb]

I think ?? should be typed with second operand type, without unification, but currently it does not.
So i changed make_if_then_else ctx e e1 e2 with_type p back to

let iftype = WithType.WithType(e2.etype,None) in
make_if_then_else ctx e e1 e2 if_type p

But there is null-safety error, if i pass nullVar ?? 2 to not-nullable function arg (latest main line).
Not sure if this is null-safety bug or i should do something else with typing.
Without this change i get Null<Int> instead of Int for $type(notNull);
There is also some missed errors for ?: expressions, it seems?
Any help would be nice.

@:nullSafety(StrictThreaded)
class Main {
	static function main() {
		final nullInt:Null<Int> = null;

		final notNull = nullInt ?? 2;
		final notNull2:Int = nullInt ?? 2;
		$type(notNull); // should be Int
		$type(notNull2); // should be Int

		final isNull = nullInt == null ? 2 : nullInt;
		final isNull2:Int = nullInt == null ? 2 : nullInt;
		$type(isNull); // should be Null<Int>
		$type(isNull2); // should be Int (with null-safety error?)

		eq(nullInt == null ? 2 : nullInt, 2); // should be null-safety error?
		eq(nullInt ?? 2, 2); // but it is there with if_type based on e2.etype
	}

	static function eq<T>(a:T, b:T):Void {
		trace(a == b);
	}
}

If iftype approach is good, some context:
nullInt ?? 2 Expression looks like that:

@:mergeBlock {
	var tmp = {
	        nullInt;
	};
	if (tmp != null) tmp else {
	        2;
	};
}

and then counts as nullable because of body_callback if_body; in method process_if.
I think tmp from condition is not affected by add_to_safety or safety saved in wrong scope. @RealyUniqueName ?

[RblSb]

Bug cases for current Haxe:

@:nullSafety(StrictThreaded)
class Main {
	static function main() {
		final arr:Array<Int> = [ // Cannot use nullable value of Int as an item in Array<Int>
			{
				var tmp = (1 : Null<Int>);
				if (tmp != null) tmp else 2;
			}
		];
		arr.push(
			{ // Cannot pass nullable value to not-nullable argument "x" of function "push"
				var tmp = (1 : Null<Int>);
				if (tmp != null) tmp else 2;
			}
		);
	}
}

Only for Strict/StrictThreaded, tmp is not detected as variable and stay with STClosure scope. Can be related to:
https://github.com/HaxeFoundation/haxe/blob/development/src/typing/nullSafety.ml#L1061
But that thing does not call local_safety#declare_var anyway, something like check_expr arg/check_expr arr_item would help, but breaks some Strict tests. Don't understand much why check_expr called before or after in expression checks, sigh.

[RealyUniqueName]

I've pushed a fix for that sample to development. Try reverting your changes of nullSafety.ml and merge development.

[RblSb]

Currently i see these problems:

final a = 1;
final b = 2;
trace(a ?? b);
      ~

Will return characters 9-10 : On static platforms, null can't be used as basic type Int on static targets.
This error should be less verbose for ?? operator, but i don't see good solution to detect operator case here. Maybe skipping this check when null safety is enabled and making such check implementation in null safety filter with better context errors (You cannot compare basic type Int with null)? Don't think this is too critical for now, tho.

And another problem is disabled operator overloading. Not sure about reason, is this possible with current basic operator implementation? Cannot find where this thing is handled, because there is no errors for @:op(a ?? b).

Anyway, maybe current state of PR is okay to merge, if feature will be approved and i don't skip some broken behavior.

[RealyUniqueName]

I think on static platform a ?? b for non-nullable types of a should generate just a

[RblSb]

If you forgot to write Null<T> signature for json field and read it like a ?? 0, you will get different runtime behavior on static targets (probably just NPE), as i understand.
So i guess there is should be error or warning, and probably for all targets? And how safe the runtime we want?
There is warnings from kotlin/dart, as reference:
Elvis operator (?:) always returns the left operand of non-nullable type String
The left operand can't be null, so the right operand is never executed.

[RealyUniqueName]

If you forgot to write Null signature for json field and read it like a ?? 0, you will get different runtime behavior on static targets (probably just NPE), as i understand.

I don't get what you mean. Could you please provide a sample?

[RblSb]

var a = cast null;
// or
var a = ({} : Dynamic).x;
var b = a ?? 2; // Error: On static platforms, null can't be used as basic type Int

a type is Unknown at this point, and if we generate a ?? 2 as a, then there is no error, and different results (0 or 2).
If a; generation is fine, there is should be a warning.
Should this warning be displayed for all targets?

[RealyUniqueName]

Then ?? generation could be changed.
If it's currently

var tmp = a;
if(tmp == null) b else a;

just change the type of tmp to Null<monomorph>.

[RblSb]

Is this would allocate new tmp objects on static targets, like hxcpp?

[RblSb]

class Main {
	static function main() {
		final a = Std.random(0);

		final b = {
			final tmp:Null<Int> = a;
			if (tmp != null) tmp else 2;
		}
		logInt(b);

		final c = {
			final tmp:Int = a;
 			// silly unspecified hack
			if (tmp != cast null) tmp else 2;
		}
		logInt(c);
	}

	static function logInt(a:Int):Void trace(a);
}

hxcpp:

void Main_obj::main(){
            	HX_STACKFRAME(&_hx_pos_e47a9afac0942eb9_2_main)
HXLINE(   3)		int a = ::Std_obj::random(0);
HXLINE(   6)		 ::Dynamic tmp = a;
HXLINE(   5)		 ::Dynamic b;
HXLINE(   7)		if (::hx::IsNotNull( tmp )) {
HXLINE(   5)			b = tmp;
            		}
            		else {
HXLINE(   5)			b = 2;
            		}
HXLINE(   9)		::Main_obj::logInt(( (int)(b) ));
HXLINE(  11)		int c;
HXLINE(  13)		if (::hx::IsNotNull( a )) {
HXLINE(  11)			c = a;
            		}
            		else {
HXLINE(  11)			c = 2;
            		}
HXLINE(  15)		::Main_obj::logInt(c);
            	}

[RealyUniqueName]

Is this would allocate new tmp objects on static targets, like hxcpp?

If a is of a nullable type (String, Null<Int> etc) - no.
If a is not nullable (Int, Float etc) - yes.

[Simn]

Are there any TODOs here or can we head towards merging this?

[RblSb]

I would like to add The left operand can't be null, so the right operand is never executed warning for stuff like notNullA ?? b, so retyping notNullA to Null<T> would be visible, since it has some overhead, but this addition depends on #7736 and main implementation will not be changed, i think. Dont see other problems currently.

[Simn]

@RealyUniqueName The changes look good to me. Since you're Mr. Null, could you double-check that all cases are covered in the tests? If so, we can merge this.

[Simn]

I'll merge this now. If there's some fallout, we can deal with that separately.

Thank you for this contribution!