Skip to content
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

Should some object-specific operations be moved out of GcObject? #1323

Closed
jasonwilliams opened this issue Jun 15, 2021 · 6 comments
Closed
Labels
E-Medium Medium difficulty problem question Further information is requested technical debt

Comments

@jasonwilliams
Copy link
Member

At some point (when working on the stack) we will need to operate on objects which are not GC managed. These would be objects that can live on the stack.

We have some functionality (such as call_construct) living in GcObject and not in Object, does anyone know why this is?
My expectation is that all object functionality lives in Object, then GCObject acts as a GC wrapper (plus only has GC related functionality)

@jasonwilliams jasonwilliams added question Further information is requested technical debt E-Medium Medium difficulty problem labels Jun 15, 2021
@jedel1043
Copy link
Member

jedel1043 commented Jul 28, 2021

I agree with the sentiment. It is confusing that you can call some funcions directly from GcObject but can't call several others without borrow(). However, methods like

pub fn get<K>(&self, key: K, context: &mut Context) -> Result<Value>
where
K: Into<PropertyKey>,
{
// 1. Assert: Type(O) is Object.
// 2. Assert: IsPropertyKey(P) is true.
// 3. Return ? O.[[Get]](P, O).
self.__get__(&key.into(), self.clone().into(), context)
}
that self clone the Gc reference makes refactoring a bit complicated. We have several options to solve this:

  • Move all Object methods to GcObject. Now every method is centralized on a single place but this doesn't solve the problem about objects living on the stack.

  • Separate internal object methods ([[get]], [[set]], OrdinaryGet, OrdinarySet, etc.) from object operations (Get, Set, CreateProperty). Internal methods would live on Object, while object operations would live on GcObject. However, this doesn't solve the problem about objects that live on the stack, so when those arrive we would need to refactor GcObject again to extract the operations.

  • The same solution as the previous one, but ditch the GcObject internal methods and create free functions that take object references as arguments. Maybe possibly create an ObjectRef trait that GcObject and StackObject implement and make the functions generic. This solution follows more closely the spec and has the advantage of being very easy to refactor when stack objects arrive.

@HalidOdat
Copy link
Member

HalidOdat commented Jul 28, 2021

At some point (when working on the stack) we will need to operate on objects which are not GC managed. These would be objects that can live on the stack.

We have some functionality (such as call_construct) living in GcObject and not in Object, does anyone know why this is?
My expectation is that all object functionality lives in Object, then GCObject acts as a GC wrapper (plus only has GC related functionality)

I don't think we can have this, since the way that objects are used implies they need to live into a well known location. For example if we create a object on the stack and clone it, first of all objects should shallow copy, and the object are not the same, even through they should, objects should only check if the pointer is the same not content.

Even with call_construct we need to clone object to put it in environments. To even use an object in javascript it has to be converted to a value which has to be on the heap anyway.

Putting it on the stack would complicate and make some spec functionality impossible, that is why I moved operations to GcObject (#835)

@jedel1043
Copy link
Member

At some point (when working on the stack) we will need to operate on objects which are not GC managed. These would be objects that can live on the stack.

We have some functionality (such as call_construct) living in GcObject and not in Object, does anyone know why this is?
My expectation is that all object functionality lives in Object, then GCObject acts as a GC wrapper (plus only has GC related functionality)

I don't think we can have this, since the way that objects are used implies they need to live into a well known location. For example if we create a object on the stack and clone it, first of all objects should shallow copy, and the object are not the same, even through they should, objects should only check if the pointer is the same not content.

Even with call_construct we need to clone object to put it in environments. To even use an object in javascript it has to be converted to a value which has to be on the heap anyway.

Putting it on the stack would complicate and make some spec functionality impossible, that is why I moved operations to GcObject (#835)

Even if we don't implement stack objects, I think the semantics of GcObject should be closer to the idea of a fat pointer than a wrapper for an Object.

@HalidOdat
Copy link
Member

Even if we don't implement stack objects, I think the semantics of GcObject should be closer to the idea of a fat pointer than a wrapper for an Object.

Could you elaborate more on this? GcObject is a pointer, it contains a Gc<Object>

@jedel1043
Copy link
Member

Even if we don't implement stack objects, I think the semantics of GcObject should be closer to the idea of a fat pointer than a wrapper for an Object.

Could you elaborate more on this? GcObject is a pointer, it contains a Gc<Object>

The current GcObject doesn't exactly act as a pointer. You have some methods that should be callable as object.borrow().method() but are called as object.method(). Ideally you should only be able to call borrow and borrow_mut on GcObject for it to be just a fat pointer, that way you don't need to decide if you should call borrow or not when implementing a method related to Object.

@jedel1043
Copy link
Member

We follow the spec more closely now. Most of the JsObject operations are for the Object type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-Medium Medium difficulty problem question Further information is requested technical debt
Projects
None yet
Development

No branches or pull requests

3 participants