improving nullability flow checking

[trusktr]

From #1972 (comment)

[dcodeIO]

Can you provide some context? In particular, where would this be safe, since without a runtime check we'd lose type safety and as such would introduce potential hazards?

[jtenner]

Hey does this already work with --no-assert?

Context: I've already done a pointer check and don't need to perform the actual assertion

[trusktr]

@willemneal What was the idea here? Maybe it is not possible to remove the runtime aspect?

[willemneal]

Consider:

function stringOrNull(s: string | null): string | null { return s; }

let s = stringOrNull("hello")!; // The type system should know this is non-null, but the `!` confirms it.
let n = stringOrNull(null)!; // Not safe

With the proposed option for ! the first case above it is safe, while the second isn't. However, this is also true for typescript. The idea here is that by default ! performs a runtime check to ensure that it's safe, but in the cases where the author knows better than the compiler it can help

As for @jtenner's point, that would work, but it might be the case that the author wants bounds checks or other assertions but not this particular one.

[dcodeIO]

The difference to TS here is that if an author is wrong, they won't be greeted with a "Cannot do X on null" runtime error but with a potential hazard corrupting memory, leading to undefined behavior, a broken runtime or silent security issues.

[willemneal]

True, but it's not the default semantics and the author is responsible.

And it is possible currently to make the following function to achieve the same result.

@inline
function isNotNull<T>(t: T): NonNullable<T> { return t; }

let s = isNotNull(stringOrNull("hello")); 
// or
let s = <string>stringOrNull("hello");

So really the change is a semantic sugar of the above approaches.

[dcodeIO]

Wait, are you saying that the use of NonNullable<T> is unsafe?

Edit: No, it's not, that results in

  ERROR TS2322: Type '~lib/string/String | null' is not assignable to type '~lib/string/String'.

   function isNotNull<T>(t: T): NonNullable<T> { return t; }

and the second inserts a runtime check.

[willemneal]

Sorry it's

   function isNotNull<T>(t: T): NonNullable<T> { return <NonNullable<T>>t; }

and I updated my previous comment to remove the !.

And it's only unsafe if you use it in a situation where it could be null.

[dcodeIO]

That's similar to <string>stringOrNull("hello"). The cast there inserts a runtime check as well to be safe.

[willemneal]

Oh I didn't know that casting resulted in a runtime check.

Here is the NonNullable in safe place to allow for the next generic call to not have the null value in the type:
https://github.com/gagdiez/serial-as/blob/4a397fdd5e1bc6d303136f03912641cf00254d21/borsh/assembly/serializer.ts#L37

[dcodeIO]

In this case the compiler knows that t is never null and can omit the runtime check.

[willemneal]

Technically it shouldn't check in the first example since it should narrow the return type.

[dcodeIO]

TS doesn't do that, though:

function stringOrNull(s: string | null): string | null { return s; }
stringOrNull("asd").length; // Object is possibly 'null'.(2531)

[willemneal]

Ah shoot but it should!

I hate null and always have. Perhaps we should look into multivalue return for a standard Option type.