Rework the class system

[DelSkayn]

This PR changes how the classes work in rquickjs.

Currently rquickjs uses the internal QuickJS class registration system to handle rust classes.
This requires some way to store class-id's per type.
In the existing code this is done with a per type static global which is initialized once and then reused.
This works because QuickJS class ids are global and don't depend on the runtime.

However this approach has some problems.
First in QuickJS-NG the class-ids are no longer global which breaks this approach.
Second the class-ids globals are not actually unique per type but per implementation of the JsClass trait.
This causes problems for generic types like in #340.

This PR instead uses only two QuickJS classes registered during initialization: rust-class and callable-rust-class.
These classes are defined to use a v-table stored alongside allocated rust class objects to handle finalization, tracing and calling in the case of callable rust classes.
This v-table can also be used to distinguish between different classes and different types and works for generic objects fixing #340.
Prototypes are handled fully on the rust side, whenever a class is instantiated its prototype is looked up in a hashmap stored within the runtime.

[richarddd]

I really like this design - it feels much simpler and less object-oriented. While we're revisiting the class and function FFI, have you considered using polymorphism through Rust or JavaScript classes? I know it's out of scope for this PR, but I wanted to mention it so we don't make future integration more complex. I would add some test cases for extending Rust classes in JS (works in current class implementation):

class Vec4 extends Vec3 {
   w = 0
   constructor(x,y,z,w){
      super(x,y,z);
      this.w = w;
   }
}

[richarddd]

This is super nice. Once this is merged I will rebase my NG PR and we should be pretty good to go 👌

[DelSkayn]

Unfortunately it seems I made an invalid assumption which makes this PR as it is currently unsound.

It turns out that constant address are not guaranteed to be stable nor unique. So the type distinction by v-table address is unsound. See rust-lang/unsafe-code-guidelines#522. This is probably the reason why the windows test fails.

This could still work if there was a way to get store a type-id within the v-table. However TypeId::of::<T>() is not yet const and it requires T: 'static which is not compatible with the 'js lifetime safety mechanism. So we would need an extra function stored in the v-table which returns the type-id for the 'js lifetime erased type.

I am currently experimenting with adding a trait called ClassId which is unsafe to implement and has a single function id which returns the TypeId for the 'js erased type.

[richarddd]

This could still work if there was a way to get store a type-id within the v-table. However TypeId::of::<T>() is not yet const and it requires T: 'static which is not compatible with the 'js lifetime safety mechanism. So we would need an extra function stored in the v-table which returns the type-id for the 'js lifetime erased type.

What about using the approach i did to store an AtomicUsize in a TypeId struct with a const new function and initialize the counter via fetch_update? That way we get a thread safe alternative to TypeId::of::<T>() that has a static reference:

rquickjs/core/src/class/id.rs

Lines 8 to 10 in 6cf0761

	pub struct ClassId {
	type_id: AtomicU32,
	}

rquickjs/core/src/class/id.rs

Lines 35 to 43 in 6cf0761

	fn init_type_id(&self) -> u32 {
	let id = self.type_id.load(Ordering::Relaxed);
	if id == 0 {
	let new_id = CLASS_ID_COUNTER.fetch_add(1, Ordering::Relaxed);
	self.type_id.store(new_id, Ordering::Relaxed);
	return new_id;
	}
	id
	}

[DelSkayn]

What about using the approach i did to store an AtomicUsize in a TypeId struct with a const new function and initialize the counter via fetch_update? That way we get a thread safe alternative to TypeId::of::() that has a static reference:

Does this still use the old approach of having JsClass have a function fn class_id() -> &'static ClassId which returns the static reference? Because this approach is broken. It doesn't return a static reference per type but per implementation so if you do.

struct Generic<T>(T);

impl<'js,T> JsClass<'js> for Generic<T> {
    const NAME: &'static str = "Generic";
    
    fn class_id() -> &'static crate::class::ClassId {
        static ID: ClassId = ClassId::new();
        &ID
    }

    fn prototype(ctx: &Ctx<'js>) -> Result<Option<Object<'js>>> {
       // .. omit other functions.
    }
}

Then

std::ptr::eq(<Generic<usize> as JsClass>::class_id(), <Generic<u8> as JsClass>::class_id())

Returns true because all Generic<T> share the same implementation. So we can't rely on this to ensure two types are the same. See #340

[richarddd]

What about using the approach i did to store an AtomicUsize in a TypeId struct with a const new function and initialize the counter via fetch_update? That way we get a thread safe alternative to TypeId::of::() that has a static reference:

Does this still use the old approach of having JsClass have a function fn class_id() -> &'static ClassId which returns the static reference? Because this approach is broken. It doesn't return a static reference per type but per implementation so if you do.

struct Generic<T>(T);

impl<'js,T> JsClass<'js> for Generic<T> {
    const NAME: &'static str = "Generic";
    type Mutable = Readable;
    const CALLABLE: bool = true;
    
    fn class_id() -> &'static crate::class::ClassId {
        static ID: ClassId = ClassId::new();
        &ID
    }

    fn prototype(ctx: &Ctx<'js>) -> Result<Option<Object<'js>>> {
       // .. omit other functions.
    }
}

Then

std::ptr::eq(<Generic<usize> as JsClass>::class_id(), <Generic<u8> as JsClass>::class_id())

Returns true because all Generic<T> share the same implementation. So we can't relay on this to ensure two types are the same.

Ah right, i didn't consider generics. Can we transmute(TypeId::of::<T>()) to get rid of static?

pub struct TypeId {
    pub val: (u64, u64),
}
impl TypeId {
    pub fn of<T: Any>() -> Self {
        unsafe { mem::transmute(TypeId::of::<T>()) }
    }
}

[DelSkayn]

Ah right, i didn't consider generics. Can we transmute(TypeId::of::()) to get rid of static?

The type id object itself is not the problem, the problem is in that to get the type id of T T needs to be 'static as Any has : 'static as a trait bound. So if you have a type which contains an rquickjs object like Container<'js>(Object<'js>) you need that 'js lifetime on the container type which in turn means it isn't 'static which means we can't get the type id of that type.

So you wouldn't be able to obtain a Class<'js,Container<'js>> as we can't have none static types if we directly rely on Type::of::<T>() to get the type id.

[DelSkayn]

I have an approach where I require all implementers of JsClass to also implement Outlive which now has the definition of:

pub unsafe trait Outlive<'js> {
    /// The target which has the same type as a `Self` but with another lifetime `'t`
    type Target<'to>: 'to;
}

Outlive was previously used to erase the lifetime of types in Persistant so that the could be handled outside of the with closure. The only change is the extra 'to bound which any type which implemented this trait should fit. If all classes implement this trait you can just do TypeId::of::<Cls::Target<'static>>() to get the type id.

This will however need support from the macros to properly implement and it might be a bit difficult to do this in a way that doesn't allow unsound implementations of Outlive. Outlive must only change the lifetime of rquickjs types and types which derive a bound from those types. If any other lifetime is changed it might cause unsound casting to happen.

[richarddd]

Some minor comments

[richarddd]

RustFunction => __qjs_rf

[richarddd]

Should we also use a more private name, i.e __qjs_rc and put this as a const some where?

[DelSkayn]

We can, but it doesn't really matter. RustClass is not accessable anywhere, it is not stored in the global object or anywhere else so it can't pollute a namespace.
It also can't cause conflicts as you can register two classes with the same class_name without problems.

The only place you will see this name is in dumps when enabling the debug dump features. It might be a good idea to make this a bit more descriptive, maybe RquickjsClass to clearly signify where the class came from.

[richarddd]

So think i figured out a great way to solve this without hashtable:

1. Each vtable gets an additional auto incrementing index_id (derived form an atomic u32)
2. prototypes are stored as a Vec (or TinyVec)
3. Proto objects are stored and accessed via the array index of index_id for the vtable

Fast, constant time lookup, no hashing or collitions

[DelSkayn]

That unfortunately doesn't work the trick I use for generating a vtable for each type doesn't allow mutable state within that vtable. If you try to add any type which can mutate, like AtomicU32, UnsafeCell, and others it will cause a compile error.

So there is no way to get an auto incrementing value. You can't generate one within a const environment and you can't store one during runtime in a vtable like static.

[richarddd]

I experimented with top level recursion but it doest work for this use case. We'll have to stick with hashtable which i think is fine :)

[richarddd]

So think i figured out a great way to solve this without hashtable:

1. Each vtable gets an additional auto incrementing index_id (derived form an atomic u32)
2. prototypes are stored as a Vec (or TinyVec)
3. Proto objects are stored and accessed via the array index of index_id for the vtable

Fast, constant time lookup, no hashing or collitions