Simplify the number unboxing logic #43271
Conversation
lewing
added
arch-wasm
lewing
force-pushed
the
wasm-unbox-number
branch
from
7356423 to
4d7e20a
Compare
|
This stops being a good idea once i64/u64 support is added, though, because those aren't Number |
|
longs will have to be handled specially in any case. This preserves the current behavior while simplifying code. |
lewing
force-pushed
the
wasm-unbox-number
branch
from
63faddc to
576cd53
Compare
....Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs
Show resolved
Hide resolved
| if (d >= int.MinValue && d <= int.MaxValue) | ||
| return (int)d; | ||
| else if (d >= uint.MinValue && d <= uint.MaxValue) | ||
| return (uint)d; |
There was a problem hiding this comment.
I don't like this. We're arbitrarily deciding whether a number should be signed or not based on ranges, this is going to cause reflective code like x = (int)obj to break for no obvious reason. I guess we could tell the end-user to just use Convert.xxx methods...
There was a problem hiding this comment.
I don't like it either, this works in more cases than the current code.
There was a problem hiding this comment.
To clarify, this code is only called by the number boxing code on the js side which does an even less effective check and would require another method binding to achieve the same result (while this removes one),
|
blocked against #41808 which is prefered |
ghost
locked as resolved and limited conversation to collaborators
From the js binding perspective all of these types are numbers (see the method binding) so treat them that way.