avm2: Store PropertyClass in `VtableData`

Calling `get_trait` copies the returned `Property`, so the caching
we performed in `PropertyClass` was never actually getting used.

Instead, we now store our `PropertyClass` values in a `Vec`
indexed by slot id. `set_property` and `init_property` now perform
coercions by going through the `VTable,` which writes the updated
`PropertyClass` back into the array.
This commit is contained in:
Aaron Hill 2022-07-30 19:58:50 -05:00 committed by GitHub
parent 01a0d702af
commit 2ce6679643
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 90 additions and 56 deletions

View File

@ -142,8 +142,9 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
activation: &mut Activation<'_, 'gc, '_>, activation: &mut Activation<'_, 'gc, '_>,
) -> Result<Value<'gc>, Error> { ) -> Result<Value<'gc>, Error> {
match self.vtable().and_then(|vtable| vtable.get_trait(multiname)) { match self.vtable().and_then(|vtable| vtable.get_trait(multiname)) {
Some(Property::Slot { slot_id, class: _ }) Some(Property::Slot { slot_id }) | Some(Property::ConstSlot { slot_id }) => {
| Some(Property::ConstSlot { slot_id, class: _ }) => self.base().get_slot(slot_id), self.base().get_slot(slot_id)
}
Some(Property::Method { disp_id }) => { Some(Property::Method { disp_id }) => {
if let Some(bound_method) = self.get_bound_method(disp_id) { if let Some(bound_method) = self.get_bound_method(disp_id) {
return Ok(bound_method.into()); return Ok(bound_method.into());
@ -197,8 +198,11 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
activation: &mut Activation<'_, 'gc, '_>, activation: &mut Activation<'_, 'gc, '_>,
) -> Result<(), Error> { ) -> Result<(), Error> {
match self.vtable().and_then(|vtable| vtable.get_trait(multiname)) { match self.vtable().and_then(|vtable| vtable.get_trait(multiname)) {
Some(Property::Slot { slot_id, mut class }) => { Some(Property::Slot { slot_id }) => {
let value = class.coerce(activation, value)?; let value = self
.vtable()
.unwrap()
.coerce_trait_value(slot_id, value, activation)?;
self.base_mut(activation.context.gc_context).set_slot( self.base_mut(activation.context.gc_context).set_slot(
slot_id, slot_id,
value, value,
@ -247,9 +251,11 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
activation: &mut Activation<'_, 'gc, '_>, activation: &mut Activation<'_, 'gc, '_>,
) -> Result<(), Error> { ) -> Result<(), Error> {
match self.vtable().and_then(|vtable| vtable.get_trait(multiname)) { match self.vtable().and_then(|vtable| vtable.get_trait(multiname)) {
Some(Property::Slot { slot_id, mut class }) Some(Property::Slot { slot_id }) | Some(Property::ConstSlot { slot_id }) => {
| Some(Property::ConstSlot { slot_id, mut class }) => { let value = self
let value = class.coerce(activation, value)?; .vtable()
.unwrap()
.coerce_trait_value(slot_id, value, activation)?;
self.base_mut(activation.context.gc_context).set_slot( self.base_mut(activation.context.gc_context).set_slot(
slot_id, slot_id,
value, value,
@ -303,8 +309,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
activation: &mut Activation<'_, 'gc, '_>, activation: &mut Activation<'_, 'gc, '_>,
) -> Result<Value<'gc>, Error> { ) -> Result<Value<'gc>, Error> {
match self.vtable().and_then(|vtable| vtable.get_trait(multiname)) { match self.vtable().and_then(|vtable| vtable.get_trait(multiname)) {
Some(Property::Slot { slot_id, class: _ }) Some(Property::Slot { slot_id }) | Some(Property::ConstSlot { slot_id }) => {
| Some(Property::ConstSlot { slot_id, class: _ }) => {
let obj = self.base().get_slot(slot_id)?.as_callable( let obj = self.base().get_slot(slot_id)?.as_callable(
activation, activation,
Some(multiname), Some(multiname),

View File

@ -11,22 +11,11 @@ use gc_arena::{Collect, Gc};
#[derive(Debug, Collect, Clone, Copy)] #[derive(Debug, Collect, Clone, Copy)]
#[collect(no_drop)] #[collect(no_drop)]
pub enum Property<'gc> { pub enum Property {
Virtual { Virtual { get: Option<u32>, set: Option<u32> },
get: Option<u32>, Method { disp_id: u32 },
set: Option<u32>, Slot { slot_id: u32 },
}, ConstSlot { slot_id: u32 },
Method {
disp_id: u32,
},
Slot {
slot_id: u32,
class: PropertyClass<'gc>,
},
ConstSlot {
slot_id: u32,
class: PropertyClass<'gc>,
},
} }
/// The type of a `Slot`/`ConstSlot` property, represented /// The type of a `Slot`/`ConstSlot` property, represented
@ -42,7 +31,7 @@ pub enum Property<'gc> {
/// Additionally, property class resolution uses special /// Additionally, property class resolution uses special
/// logic, different from normal "runtime" class resolution, /// logic, different from normal "runtime" class resolution,
/// that allows private types to be referenced. /// that allows private types to be referenced.
#[derive(Debug, Collect, Clone, Copy)] #[derive(Debug, Collect, Clone)]
#[collect(no_drop)] #[collect(no_drop)]
pub enum PropertyClass<'gc> { pub enum PropertyClass<'gc> {
/// The type `*` (Multiname::is_any()). This allows /// The type `*` (Multiname::is_any()). This allows
@ -61,24 +50,28 @@ impl<'gc> PropertyClass<'gc> {
) -> Self { ) -> Self {
PropertyClass::Name(Gc::allocate(activation.context.gc_context, (name, unit))) PropertyClass::Name(Gc::allocate(activation.context.gc_context, (name, unit)))
} }
/// Returns `value` coerced to the type of this `PropertyClass`.
/// The bool is `true` if this `PropertyClass` was just modified
/// to cache a class resolution, and `false` if it was not modified.
pub fn coerce( pub fn coerce(
&mut self, &mut self,
activation: &mut Activation<'_, 'gc, '_>, activation: &mut Activation<'_, 'gc, '_>,
value: Value<'gc>, value: Value<'gc>,
) -> Result<Value<'gc>, Error> { ) -> Result<(Value<'gc>, bool), Error> {
let class = match self { let (class, changed) = match self {
PropertyClass::Class(class) => Some(*class), PropertyClass::Class(class) => (Some(*class), false),
PropertyClass::Name(gc) => { PropertyClass::Name(gc) => {
let (name, unit) = &**gc; let (name, unit) = &**gc;
let outcome = resolve_class_private(name, *unit, activation)?; let outcome = resolve_class_private(name, *unit, activation)?;
let class = match outcome { let class = match outcome {
ResolveOutcome::Class(class) => { ResolveOutcome::Class(class) => {
*self = PropertyClass::Class(class); *self = PropertyClass::Class(class);
Some(class) (Some(class), true)
} }
ResolveOutcome::Any => { ResolveOutcome::Any => {
*self = PropertyClass::Any; *self = PropertyClass::Any;
None (None, true)
} }
ResolveOutcome::NotFound => { ResolveOutcome::NotFound => {
// FP allows a class to reference its own type in a static initializer: // FP allows a class to reference its own type in a static initializer:
@ -103,7 +96,9 @@ impl<'gc> PropertyClass<'gc> {
&& unit.map(|u| u.domain()) && unit.map(|u| u.domain())
== Some(class.class_scope().domain()) == Some(class.class_scope().domain())
{ {
return Ok(value); // Even though resolution succeeded, we haven't modified
// this `PropertyClass`, so return `false`
return Ok((value, false));
} }
} }
} }
@ -115,15 +110,15 @@ impl<'gc> PropertyClass<'gc> {
}; };
class class
} }
PropertyClass::Any => None, PropertyClass::Any => (None, false),
}; };
if let Some(class) = class { if let Some(class) = class {
value.coerce_to_type(activation, class) Ok((value.coerce_to_type(activation, class)?, changed))
} else { } else {
// We have a type of '*' ("any"), so don't // We have a type of '*' ("any"), so don't
// perform any coercions // perform any coercions
Ok(value) Ok((value, changed))
} }
} }
} }
@ -191,7 +186,7 @@ fn resolve_class_private<'gc>(
} }
} }
impl<'gc> Property<'gc> { impl Property {
pub fn new_method(disp_id: u32) -> Self { pub fn new_method(disp_id: u32) -> Self {
Property::Method { disp_id } Property::Method { disp_id }
} }
@ -210,11 +205,11 @@ impl<'gc> Property<'gc> {
} }
} }
pub fn new_slot(slot_id: u32, class: PropertyClass<'gc>) -> Self { pub fn new_slot(slot_id: u32) -> Self {
Property::Slot { slot_id, class } Property::Slot { slot_id }
} }
pub fn new_const_slot(slot_id: u32, class: PropertyClass<'gc>) -> Self { pub fn new_const_slot(slot_id: u32) -> Self {
Property::ConstSlot { slot_id, class } Property::ConstSlot { slot_id }
} }
} }

View File

@ -27,7 +27,11 @@ pub struct VTableData<'gc> {
protected_namespace: Option<Namespace<'gc>>, protected_namespace: Option<Namespace<'gc>>,
resolved_traits: PropertyMap<'gc, Property<'gc>>, resolved_traits: PropertyMap<'gc, Property>,
/// Stores the `PropertyClass` for each slot,
/// indexed by `slot_id`
slot_classes: Vec<PropertyClass<'gc>>,
method_table: Vec<ClassBoundMethod<'gc>>, method_table: Vec<ClassBoundMethod<'gc>>,
@ -54,6 +58,7 @@ impl<'gc> VTable<'gc> {
scope: None, scope: None,
protected_namespace: None, protected_namespace: None,
resolved_traits: PropertyMap::new(), resolved_traits: PropertyMap::new(),
slot_classes: vec![],
method_table: vec![], method_table: vec![],
default_slots: vec![], default_slots: vec![],
}, },
@ -64,7 +69,7 @@ impl<'gc> VTable<'gc> {
VTable(GcCell::allocate(mc, self.0.read().clone())) VTable(GcCell::allocate(mc, self.0.read().clone()))
} }
pub fn get_trait(self, name: &Multiname<'gc>) -> Option<Property<'gc>> { pub fn get_trait(self, name: &Multiname<'gc>) -> Option<Property> {
self.0 self.0
.read() .read()
.resolved_traits .resolved_traits
@ -72,6 +77,26 @@ impl<'gc> VTable<'gc> {
.cloned() .cloned()
} }
/// Coerces `value` to the type of the slot with id `slot_id`
pub fn coerce_trait_value(
&self,
slot_id: u32,
value: Value<'gc>,
activation: &mut Activation<'_, 'gc, '_>,
) -> Result<Value<'gc>, Error> {
// Drop the `write()` guard, as 'slot_class.coerce' may need to access this vtable.
let mut slot_class = { self.0.read().slot_classes[slot_id as usize].clone() };
let (value, changed) = slot_class.coerce(activation, value)?;
// Calling coerce modified `PropertyClass` to cache the class lookup,
// so store the new value back in the vtable.
if changed {
self.0.write(activation.context.gc_context).slot_classes[slot_id as usize] = slot_class;
}
Ok(value)
}
pub fn has_trait(self, name: &Multiname<'gc>) -> bool { pub fn has_trait(self, name: &Multiname<'gc>) -> bool {
self.0 self.0
.read() .read()
@ -175,6 +200,7 @@ impl<'gc> VTable<'gc> {
if let Some(superclass_vtable) = superclass_vtable { if let Some(superclass_vtable) = superclass_vtable {
write.resolved_traits = superclass_vtable.0.read().resolved_traits.clone(); write.resolved_traits = superclass_vtable.0.read().resolved_traits.clone();
write.slot_classes = superclass_vtable.0.read().slot_classes.clone();
write.method_table = superclass_vtable.0.read().method_table.clone(); write.method_table = superclass_vtable.0.read().method_table.clone();
write.default_slots = superclass_vtable.0.read().default_slots.clone(); write.default_slots = superclass_vtable.0.read().default_slots.clone();
@ -195,10 +221,11 @@ impl<'gc> VTable<'gc> {
} }
} }
let (resolved_traits, method_table, default_slots) = ( let (resolved_traits, method_table, default_slots, slot_classes) = (
&mut write.resolved_traits, &mut write.resolved_traits,
&mut write.method_table, &mut write.method_table,
&mut write.default_slots, &mut write.default_slots,
&mut write.slot_classes,
); );
for trait_data in traits { for trait_data in traits {
@ -299,31 +326,38 @@ impl<'gc> VTable<'gc> {
slot_id slot_id
}; };
let new_prop = match trait_data.kind() { if new_slot_id as usize >= slot_classes.len() {
// We will overwrite `PropertyClass::Any` when we process the slots
// with the ids that we just skipped over.
slot_classes.resize(new_slot_id as usize + 1, PropertyClass::Any);
}
let (new_prop, new_class) = match trait_data.kind() {
TraitKind::Slot { TraitKind::Slot {
type_name, unit, .. type_name, unit, ..
} => Property::new_slot( } => (
new_slot_id, Property::new_slot(new_slot_id),
PropertyClass::name(activation, type_name.clone(), *unit), PropertyClass::name(activation, type_name.clone(), *unit),
), ),
TraitKind::Function { .. } => Property::new_slot( TraitKind::Function { .. } => (
new_slot_id, Property::new_slot(new_slot_id),
PropertyClass::Class(activation.avm2().classes().function), PropertyClass::Class(activation.avm2().classes().function),
), ),
TraitKind::Const { TraitKind::Const {
type_name, unit, .. type_name, unit, ..
} => Property::new_const_slot( } => (
new_slot_id, Property::new_const_slot(new_slot_id),
PropertyClass::name(activation, type_name.clone(), *unit), PropertyClass::name(activation, type_name.clone(), *unit),
), ),
TraitKind::Class { .. } => Property::new_const_slot( TraitKind::Class { .. } => (
new_slot_id, Property::new_const_slot(new_slot_id),
PropertyClass::Class(activation.avm2().classes().class), PropertyClass::Class(activation.avm2().classes().class),
), ),
_ => unreachable!(), _ => unreachable!(),
}; };
resolved_traits.insert(trait_data.name(), new_prop); resolved_traits.insert(trait_data.name(), new_prop);
slot_classes[new_slot_id as usize] = new_class;
} }
} }
} }
@ -379,10 +413,10 @@ impl<'gc> VTable<'gc> {
write.default_slots.push(Some(value)); write.default_slots.push(Some(value));
let new_slot_id = write.default_slots.len() as u32 - 1; let new_slot_id = write.default_slots.len() as u32 - 1;
write.resolved_traits.insert( write
name, .resolved_traits
Property::new_slot(new_slot_id, PropertyClass::Class(class)), .insert(name, Property::new_slot(new_slot_id));
); write.slot_classes.push(PropertyClass::Class(class));
new_slot_id new_slot_id
} }