Null<T> inference

[Gama11]

Would be nice if Null<T> was inferred here, so it compiles with null safety enabled:

class Main {
	@:nullSafety
	public static function main() {
		var foo = null; // Null safety: Cannot assign nullable value here.
		$type(foo); // Unknown<0>
		foo = "";
		$type(foo); // String
	}
}

In that case it would be Null<Unknown<0>> in the first $type and Null<String> for the second.

[Simn]

@ncannasse: How do you feel about this? It seems correct to me to infer null as Null<?>, but I'm somewhat concerned about inference differences that come from different typing order:

class Main {
	static public function main() {
		var a = null;
		a = "foo";
		$type(a); // Null<String>

		var b = "foo";
		b = null;
		$type(b); // String
	}
}

I suppose this is consistent with our general unification behavior though.

[ncannasse]

@Simn atm not auto-typing things as Null<Unknown> allows us to detect and warn on static platforms null cannot be used as a basic type and requires explicit Null<Int> typing. Also, the typing order bother me here, so I would prefer to keep it as-it.

[Gama11]

Type inference is always affected by order though? For instance this doesn't seem much different to me:

class Main {
    static function main() {
        var a = [];
        a.push(new Parent());
        $type(a); // Array<Parent>
        a.push(new Child()); // works
        
        var a = [];
        a.push(new Child());
        $type(a); // Array<Child>
        a.push(new Parent()); // fails
    }
}

class Parent { public function new() {} }
class Child extends Parent { public function new() { super(); } }

[Simn]

That's the conclusion I arrived at too. It's not really different from assigning Int and Float values where the order matters as well.

I think we should infer Null<?> now. It's a bit silly that the compiler doesn't infer it but then complains about it...

[nadako]

I have looked into this today and I believe we should infer Null<?> for null, because:

1. It is consistent with other parametrized types (like the the Array example above)
2. It has actual value of making null-safety much less annoying in a common code like:

   var found = null;
   for (i in 0...10) {
       if (...) {
           found = i;
           break;
       }
   }
   if (found != null) {
       var x:Int = found;
       ...
   }

I think that the only thing it can possibly break is the null can't be used as basic type check in static platforms, because it relies on checking TConst(TNull)s typed as a basic type, however I have this to say about it:

• In my experience this check is quite annoying by itself when developing for different targets, e.g. you develop on JS and everything is fine until you compile to C#/Flash/any static target.
• It seems weird that we do specify how Null<BasicType> is converted to BasicType in general (target-specific default values), but still disallow passing null literal for BasicType. So it's not clear to me what was the intention of this check.
• The check can be still preserved even with Null<?> inference via one of these ways:
  • In the typer, check against the expected type when typing EConst(CIdent "null")). This will make some transformed code (e.g. inline functions returning null) NOT failing, but returning default values, which actually makes them consistent with non-inline versions, as well as any other code that the check currently misses.
  • In a post-typing filter that checks if TConst(TNull) is "unified" (assigned, passed as argument, etc.) with a non-nullable-basic-type. This is quite similar to how it's currently done, but it'll find more cases (e.g. inlined null typed as Null<Int> assigned to Int), which can break existing code.

I would personally just remove the check altogether because I don't see its benefits: nowadays we have @:nullSafety that is much smarter and can catch more mistakes. But if you guys don't agree, I could work on reworking it using one of the ways I described above. But I'm convinced that we should type null literals as Null<?> for the reasons stated in the beginning.

[RblSb]

Funny to see same ctx.t.tnull (mk_mono()) change that i tested myself yesterday with two linked commits here.
Typing var foo = null as Null<Unknown> will fix this too: #10147
Typing something with Null<Int/Bool/Float> has hidden allocation on static targets, as i understand, but this change would not affect old code, as this is currently disabled. @ncannasse if we type null mono exprs with Null<Unknown>, we don't need to change something when compiling such expressions to static targets, i think? Is there other reasons to keep it as it?

And some ideas about workarounds would be nice. Migration to null safety can be complicated with such holes, so maybe something like -D null-safety (or detection of @:nullSafety context) can improve null inference (also like #6825), and library authors can adapt code more softly. There is should be safe std api at some point, i hope.

[Simn]

There's some inference problem in the neko standard library of all places. I'm dodging this for now, but will have to check what's going on there and if it happens in "real" code too.

[mockey]

I just saw that haxelib didn't compile with the previous commit. There were some errors about "recursive types".

[Simn]

Yes that was the problem I'm dodging now. Although now there's some new problem...