Skip to content

avm1: Remove Gc indirection for SuperObject #20369

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/src/avm1/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ impl<'gc> Avm1Function<'gc> {
// TODO: `super` should only be defined if this was a method call (depth > 0?)
// `f[""]()` emits a CallMethod op, causing `this` to be undefined, but `super` is a function; what is it?
let zuper = this.filter(|_| !suppress).map(|this| {
let zuper = NativeObject::Super(SuperObject::new(frame, this, depth));
let zuper = NativeObject::Super(SuperObject::new(this, depth));
Object::new_with_native(frame.strings(), None, zuper).into()
});

Expand Down
51 changes: 27 additions & 24 deletions core/src/avm1/object/super_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::avm1::function::ExecutionReason;
use crate::avm1::object::{search_prototype, ExecutionName};
use crate::avm1::{NativeObject, Object, Value};
use crate::string::AvmString;
use gc_arena::{Collect, Gc};
use gc_arena::Collect;
use ruffle_macros::istr;

/// Implementation of the `super` object in AS2.
Expand All @@ -18,44 +18,47 @@ use ruffle_macros::istr;
/// with its parent class.
#[derive(Copy, Clone, Collect)]
#[collect(no_drop)]
pub struct SuperObject<'gc>(Gc<'gc, SuperObjectData<'gc>>);
pub struct SuperObject<'gc> {
/// The object present as `this` throughout the superchain.
this: Object<'gc>,

/// The prototype depth of the currently-executing method.
depth: u8,

/// Adds a niche, so that enums contaning this type can use it for their discriminant.
_niche: crate::utils::ZeroU8,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this work exactly? Shouldn't Rust reuse this space for the discriminant anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rust will never overlap a discriminant with padding, because padding can have any value (it's roughly like an invisible MaybeUninit<[u8; _]> field); adding the ZeroU8 field instead says that any SuperObject must have a zero byte at this offset.

}

impl fmt::Debug for SuperObject<'_> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.debug_struct("SuperObject")
.field("ptr", &Gc::as_ptr(self.0))
.field("this", &self.this)
.field("depth", &self.depth)
.finish()
}
}

#[derive(Clone, Collect)]
#[collect(no_drop)]
pub struct SuperObjectData<'gc> {
/// The object present as `this` throughout the superchain.
this: Object<'gc>,

/// The prototype depth of the currently-executing method.
depth: u8,
}

impl<'gc> SuperObject<'gc> {
/// Construct a `super` for an incoming stack frame.
pub fn new(activation: &mut Activation<'_, 'gc>, this: Object<'gc>, depth: u8) -> Self {
Self(Gc::new(activation.gc(), SuperObjectData { this, depth }))
pub fn new(this: Object<'gc>, depth: u8) -> Self {
Self {
this,
depth,
_niche: Default::default(),
}
}

pub fn this(&self) -> Object<'gc> {
self.0.this
self.this
}

pub fn depth(&self) -> u8 {
self.0.depth
self.depth
}

pub(super) fn base_proto(&self, activation: &mut Activation<'_, 'gc>) -> Object<'gc> {
let depth = self.0.depth;
let mut proto = self.0.this;
for _ in 0..depth {
let mut proto = self.this();
for _ in 0..self.depth() {
proto = proto.proto(activation).coerce_to_object(activation);
}
proto
Expand Down Expand Up @@ -83,8 +86,8 @@ impl<'gc> SuperObject<'gc> {
constr.as_constructor().exec(
name.into(),
activation,
self.0.this.into(),
self.0.depth + 1,
self.this().into(),
self.depth() + 1,
args,
ExecutionReason::FunctionCall,
constructor,
Expand All @@ -98,7 +101,7 @@ impl<'gc> SuperObject<'gc> {
activation: &mut Activation<'_, 'gc>,
reason: ExecutionReason,
) -> Result<Value<'gc>, Error<'gc>> {
let this = self.0.this;
let this = self.this();
let (method, depth) =
match search_prototype(self.proto(activation), name, activation, this, false)? {
Some((Value::Object(method), depth)) => (method, depth),
Expand All @@ -110,7 +113,7 @@ impl<'gc> SuperObject<'gc> {
ExecutionName::Dynamic(name),
activation,
this.into(),
self.0.depth + depth + 1,
self.depth() + depth + 1,
args,
reason,
method,
Expand Down
11 changes: 10 additions & 1 deletion core/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use gc_arena::Gc;
use gc_arena::{Collect, Gc};

pub use ruffle_macros::HasPrefixField;

Expand Down Expand Up @@ -34,3 +34,12 @@ pub unsafe trait HasPrefixField<Inner>: Sized {
unsafe { Gc::cast(gc) }
}
}

/// A `u8` which is always zero. Useful to artificially introduce niches into a struct.
#[derive(Copy, Clone, Collect, Default, Eq, PartialEq, Ord, PartialOrd, Hash)]
#[collect(require_static)]
#[repr(u8)]
pub enum ZeroU8 {
#[default]
Zero = 0,
}