Skip to content

Triple equality and unexpected result on union types #1069

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mpapierski opened this issue Jan 22, 2020 · 12 comments
Closed

Triple equality and unexpected result on union types #1069

mpapierski opened this issue Jan 22, 2020 · 12 comments

Comments

@mpapierski
Copy link

Hello!

I'm learning AssemblyScript language and I stumbled upon very strange behavior of === identity operator when combined with upper case numeric types (i.e. U32) and null values.

It seems like a function that return values of type U32 | null which are tested with foo() === null returns true for both result == 0 and result == null;. It seems like its impossible to differentiate those two cases and x === null always returns true for x values of [0, null, <U32>null, <U32>0] etc.

Example:

I have a function that reads an U32 value from bytes of Uint8Array type. If length of those bytes is less than 4, then function returns null (byte stream is invalid), otherwise it returns load<u32> casted to U32 (byte stream contains a valid U32).

export function readU32(bytes: Uint8Array): U32 | null {
    if (bytes.length < 4) {
        return null;
    }
    const number = <u32>load<u32>(bytes.dataStart);
    return <U32>number;
}

Example usage that demonstrates the issue:

let bytes = new Uint8Array(4);
bytes.fill(0);
let result = readU32(bytes);
assert(result === null); // true
assert(result === <U32>0); // true

Expected behavior would be to make result === null a false when result contains zero value, but true if result is actually a null.

If the current behavior of === is intended, what would be the workaround to differentiate those two separate values?

@MaxGraey
Copy link
Member

Hi, behaviour and differences about tripple equal described in docs. u32 | null not supported yet as well as more complex union types. Only SomeReference | null currently supported.

@MaxGraey
Copy link
Member

MaxGraey commented Jan 22, 2020

But === could be sync with TS in future. See this proposal: #856

@mpapierski
Copy link
Author

Hi, behaviour and differences about tripple equal described in docs. u32 | null not supported yet as well as more complex union types. Only SomeReference | null currently supported.

Thanks for quick reply!

I'm not expert on TS/AS but wouldn't something like:

class Uint32 { value: u32 };
function readU32(bytes: Uint8Array): Uint32 | null { ... }

work, then as a workaround?

@MaxGraey
Copy link
Member

MaxGraey commented Jan 22, 2020

Yes, that's call "boxing" so you could do something like this as workaround. But this not efficient!
Much better use shared global variable which store last error code, like:

enum Status {
   Ok = 0, // no error
   ErrorDecodeInvalid = 1,
   ErrorUnknown = 2,
   // etc
}

let lastError = Status.Ok;

function readU32(bytes: Uint8Array): u32 {
   // .... some code ....
   if (ok) {
     lastError = Status.Ok;
     return result;
   }
   // some error overwise
   lastError = Status.ErrorDecodeInvalid;
   return 0;
}

// usage

let byte = readU32(buffer);
if (lastError != Status.Ok) {
   // handle error

   // reset. But this optional
   lastError = Status.Ok;
}

Or you could use special unsigned value if you sure it can't produced by function like 0xFFFFFFFF for example or NaN in case with f64

@willemneal
Copy link
Contributor

@mpapierski Another work around (not sure your use case) is to use a signed 64-bit number and if it is negative you have an error, otherways you can cast it back to a 32-bit.

Or you can do something like this: https://github.com/WebAssemblyOS/wasmos/blob/master/packages/assemblyscript/assembly/wasa/index.ts

Though in that example the result class should use a singleton object instead of allocating a fresh one each time.

@mpapierski
Copy link
Author

mpapierski commented Feb 3, 2020

@MaxGraey @willemneal thanks for replies! Of course still hoping one day uppercase numbers could became nullable.

@dcodeIO Another question which I think could be related to this: I just updated asc compiler to 0.9.1 and suddenly I can't use union types and uppercase numbers at all:

Simplified code:

export function readU32(bytes: Uint8Array): U32 | null {
    if (bytes.length < 4) {
        return null;
    }
    const number = <u32>load<u32>(bytes.dataStart);
    return <U32>number;
}

Under 0.9.1 throws following error:

ERROR TS2322: Type 'u32' is not assignable to type '~lib/number/U32'.

     return <U32>number;

It's confusing and the code looks valid (at least under 0.8.1 it worked). If it's breaking change, then what would be the new syntax for such cast?

Edit: new behaviour makes it also hard to test: readU32(...) == <U8>123 does not work as I can't create U8 literal.

@MaxGraey
Copy link
Member

MaxGraey commented Feb 3, 2020

@mpapierski
You should use changetype<U32>(number) instead <U32>number. Now this more strictly required for such unsafe casting

@mpapierski
Copy link
Author

@mpapierski
You should use changetype<U32>(number) instead <U32>number. Now this more strictly required for such unsafe casting

Ahh right! It seems to work now. Again, thanks for fast response.

@MaxGraey
Copy link
Member

MaxGraey commented Feb 3, 2020

Also you could simplify it to:

@inline
export function readU32(bytes: Uint8Array): U32 | null {
    return bytes.length < 4 ? null : load<U32>(bytes.dataStart);
}

but this will not work for U64, U16 end etc

@mpapierski
Copy link
Author

Also you could simplify it to:

@inline
export function readU32(bytes: Uint8Array): U32 | null {
    return bytes.length < 4 ? null : load<U32>(bytes.dataStart);
}

Good point!

but this will not work for U64, U16 end etc

Just noticed it:

    let value = <u64>load<i64>(bytes.dataStart);
    return changetype<U64>(value);

And error:

ERROR AS202: Type 'u64' cannot be changed to type '~lib/number/U64'.

What's the difference between U32/U16/U64 etc. They seem to be implemented in a similar fashion on the AS side (sealed unmanaged classes)?

@mpapierski
You should use changetype<U32>(number) instead <U32>number. Now this more strictly required for such unsafe casting

Now it makes sense

@dcodeIO
Copy link
Member

dcodeIO commented Feb 3, 2020

The uppercase variants are wrapper classes providing the implementations of T.toString and such behind the scenes. As such, an U64 is a special class one shouldn't changetype to, while an u64 is the correct value type. From a user's perspective, using u64 as a type is almost always correct, while using U64 as a type will most likely result in something unexpected.

@stale
Copy link

stale bot commented Mar 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 4, 2020
@stale stale bot closed this as completed Mar 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants