BREAKING CHANGE: Improve null comparison #2760
Conversation
HerrCai0907
force-pushed
the
fix/null-eq
branch
from
eca239f
to
7457a31
Compare
| export function compare_null_with_operator_overload(): void { | ||
| null == "aa"; | ||
| null != "aa"; | ||
| "aa" == null; | ||
| "aa" != null; | ||
|
|
||
| null === "aa"; | ||
| null !== "aa"; | ||
| "aa" === null; | ||
| "aa" !== null; | ||
| } |
There was a problem hiding this comment.
This is something to be very careful with. Your current code is perfectly fine for String, but I have to wonder what happens with classes with operator overloads that have signatures with non-null parameters. In that case, you would need to add some null-checking code for == that's equivalent to a && b ? eqOverload(a, b) : a == b, where a and b are the pointers of the LHS and RHS respectively, and eqOverload represents the overload function. Similar null-checking code applies to !=. You could also opt to error if ==/!= is used on nullable types that have an overload without nullable parameters.
|
Whoops, sorry, I didn't realize you removed the request to review. |
I find it should handle float and integer promotion more carefully. |
BREAK_CHANGE Fixes AssemblyScript#2662
HerrCai0907
force-pushed
the
fix/null-eq
branch
from
7457a31
to
321f9c7
Compare
HerrCai0907
changed the title
| i32.const 0 | ||
| i32.const 32 | ||
| i32.eq | ||
| drop | ||
| i32.const 0 | ||
| i32.const 32 | ||
| i32.ne | ||
| drop |
There was a problem hiding this comment.
This doesn't give me a good feeling, because this means null == overloaded isn't being compiled correctly, while overloaded == null is.
There was a problem hiding this comment.
But I think it should not block this fix.
The same thing will happen when comparing new A() == new B()
We should introduce more strictly check for operator overload.
There was a problem hiding this comment.
I suppose so. I don't like having different semantics for null == vs == null as well as for A == and == A.
There was a problem hiding this comment.
I don't like having different semantics for
null ==vs== nullas well as forA ==and== A.
Agree. This patch should land after we implement better operator overload resolution.
There was a problem hiding this comment.
implement better operator overload resolution
We would still need to special-case null somehow. There needs to be some way to differentiate between a value of type Object | null and a null literal, which has the same type in this PR.
There was a problem hiding this comment.
Why we need to differentiate between a value of type Object | null and a null literal? If we want to do it, it is necessary to introduce a new primitive type.
CountBleck
changed the title
CountBleck
changed the title
Fixes #2662.