Expose predefined class_ids for efficient lookup?

[richarddd]

When checking if an object is an instance of a an added intrinsic object such a uint8 typed array it would be useful to check the class id directly.

We can access the class id via JS_GetClassID but we can't compare with anything as the already defined tags are stored in an private unnamed enum.

This is currently rather brittle in rquckjs as typed arrays are checked by looking at the constructor name in global scope and calling qjs::JS_IsInstanceOf

[saghul]

So basically have a way to know if the class ID is in this enum: https://github.com/quickjs-ng/quickjs/blob/374915ad0c38d8a948d492c6d4a735f320339a63/quickjs.c#L119

I guess the simplest way would be to add some int JS_IsIntrinsic(JSClassID class_id) perhaps?

[richarddd]

Kind of, or rather:

if(classId == JS_CLASS_INT32_ARRAY){

}

[saghul]

We could perhaps expose the whole enum... I guess it should be harmless. WDYT @bnoordhuis ?

[chqrlie]

We could perhaps expose the whole enum... I guess it should be harmless. WDYT @bnoordhuis ?

The enum is already exposed in bellard/quickjs for JS_NewTypedArray

[saghul]

Not for all class types, which I guess is what @richarddd also needs?

[richarddd]

Yeah why not? @chqrlie yes, but that won't help as i wont know the offset here class id used here:
https://github.com/bellard/quickjs/blob/6e2e68fd0896957f92eb6c242a2e048c1ef3cae0/quickjs.c#L53441C39-L53441C60

So simply expose the entire enum would be preferable and class type checks would be a breeze for integrators :)

[bnoordhuis]

We could perhaps expose the whole enum... I guess it should be harmless. WDYT @bnoordhuis ?

ABI pitfall: the enum ends with JS_CLASS_INIT_COUNT. How do we add new classes without breaking ABI?

[saghul]

With a fast moving ecosystem like JS does it make sense to make many promises (yeah, pun intended) about ABI compatibility? Just thinking out loud.

[bnoordhuis]

I'd like to see libquickjs show up in distros someday, also because that makes downstream projects' lives easier, so I'd rather not paint ourselves into a corner too quickly, ABI-wise.

Adding API functions is fair game though. For the use case @richarddd mentions, sounds like it should be enough to have JS_IsTypedArray(), JS_IsUint8Array(), etc.?

[richarddd]

Adding API functions is fair game though. For the use case @richarddd mentions, sounds like it should be enough to have JS_IsTypedArray(), JS_IsUint8Array(), etc.?

I think that works if we can expose one method for each typed array type.

My issues one can be solved by adding JS_NewTypedArray from upstream and JS_IsTypedArray()

For reference, here are are where I need to make changes.
https://github.com/DelSkayn/rquickjs/blob/master/core/src/value/typed_array.rs#L154-L164
https://github.com/DelSkayn/rquickjs/blob/master/core/src/value/typed_array.rs#L193-L200
https://github.com/DelSkayn/rquickjs/blob/master/core/src/value/typed_array.rs#L311-L318
https://github.com/DelSkayn/rquickjs/blob/master/core/src/value/typed_array.rs#L154-L164

[Sytten]

To revive a bit this conversation, we actually need the class ID too in the serde serializer/deserializer like
https://github.com/rquickjs/rquickjs-serde/blob/62d44b26efbd11b37058cb3697391a9faa23b4fa/src/de.rs#L214-L215
I didnt write the original code for this, so there might be a better way to do this (for example I am not sure I fully understand the difference between a tag and a class id). For now they are hardcoded in Rust which is not good.
Maybe we need JS_GetBoolClassID?

[bnoordhuis]

JS_IsBigInt() is what you {c,sh}ould be using on that line you link to, right?

[Sytten]

Maybe? I am not exactly sure why the original author did both a tag check and a class ID check. I have the feeling that it might be for another edge case. Is it possible that a given value would have a class id of bool/number/bigint but not the tag?