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

avm2: Do not allocate for Namespace::any() #17771

Merged
merged 1 commit into from
Sep 5, 2024
Merged
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/avm2/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ impl<'gc> Class<'gc> {
// A 'callable' class doesn't have a signature - let the
// method do any needed coercions
vec![],
Multiname::any(activation.context.gc_context),
Multiname::any(),
true,
activation.context.gc_context,
);
Expand Down
4 changes: 2 additions & 2 deletions core/src/avm2/e4x.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1706,13 +1706,13 @@ pub fn string_to_multiname<'gc>(
) -> Multiname<'gc> {
if let Some(name) = name.strip_prefix(b'@') {
if name == b"*" {
return Multiname::any_attribute(activation.gc());
return Multiname::any_attribute();
}

let name = AvmString::new(activation.context.gc_context, name);
Multiname::attribute(activation.avm2().public_namespace_base_version, name)
} else if &*name == b"*" {
Multiname::any(activation.context.gc_context)
Multiname::any()
} else {
Multiname::new(activation.avm2().public_namespace_base_version, name)
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/avm2/globals/avmplus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ fn describe_internal_body<'gc>(
continue;
}
let prop_class_name = vtable
.slot_class_name(*slot_id, activation.context.gc_context)?
.slot_class_name(*slot_id)?
.to_qualified_name_or_star(activation.context.gc_context);

let access = match prop {
Expand Down
4 changes: 2 additions & 2 deletions core/src/avm2/globals/int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,10 @@ pub fn create_class<'gc>(activation: &mut Activation<'_, 'gc>) -> Class<'gc> {
"<int instance initializer>",
vec![ParamConfig {
param_name: AvmString::new_utf8(activation.context.gc_context, "value"),
param_type_name: Multiname::any(activation.context.gc_context),
param_type_name: Multiname::any(),
default_value: Some(Value::Integer(0)),
}],
Multiname::any(activation.context.gc_context),
Multiname::any(),
true,
mc,
),
Expand Down
2 changes: 1 addition & 1 deletion core/src/avm2/globals/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub fn instance_init<'gc>(
let ns = qname
.uri()
.map(|uri| Namespace::package(uri, api_version, &mut activation.borrow_gc()))
.unwrap_or_else(|| Namespace::any(activation.context.gc_context));
.unwrap_or_else(Namespace::any);
if ns.as_uri().is_empty() {
(Some("".into()), ns)
} else {
Expand Down
6 changes: 3 additions & 3 deletions core/src/avm2/globals/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ pub fn create_i_class<'gc>(activation: &mut Activation<'_, 'gc>) -> Class<'gc> {
has_own_property,
vec![ParamConfig::optional(
"name",
Multiname::any(activation.context.gc_context),
Multiname::any(),
Value::Undefined,
)],
Multiname::new(activation.avm2().public_namespace_base_version, "Boolean"),
Expand All @@ -289,7 +289,7 @@ pub fn create_i_class<'gc>(activation: &mut Activation<'_, 'gc>) -> Class<'gc> {
is_prototype_of,
vec![ParamConfig::optional(
"theClass",
Multiname::any(activation.context.gc_context),
Multiname::any(),
Value::Undefined,
)],
Multiname::new(activation.avm2().public_namespace_base_version, "Boolean"),
Expand All @@ -299,7 +299,7 @@ pub fn create_i_class<'gc>(activation: &mut Activation<'_, 'gc>) -> Class<'gc> {
property_is_enumerable,
vec![ParamConfig::optional(
"name",
Multiname::any(activation.context.gc_context),
Multiname::any(),
Value::Undefined,
)],
Multiname::new(activation.avm2().public_namespace_base_version, "Boolean"),
Expand Down
4 changes: 2 additions & 2 deletions core/src/avm2/globals/uint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,10 @@ pub fn create_class<'gc>(activation: &mut Activation<'_, 'gc>) -> Class<'gc> {
"<uint instance initializer>",
vec![ParamConfig {
param_name: AvmString::new_utf8(activation.context.gc_context, "value"),
param_type_name: Multiname::any(activation.context.gc_context),
param_type_name: Multiname::any(),
default_value: Some(Value::Integer(0)),
}],
Multiname::any(activation.context.gc_context),
Multiname::any(),
true,
mc,
),
Expand Down
8 changes: 4 additions & 4 deletions core/src/avm2/globals/xml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ pub fn children<'gc>(
activation,
children,
Some(xml.into()),
Some(Multiname::any(activation.gc())),
Some(Multiname::any()),
)
.into())
}
Expand Down Expand Up @@ -646,7 +646,7 @@ pub fn attributes<'gc>(
activation,
attributes,
Some(xml.into()),
Some(Multiname::any_attribute(activation.gc())),
Some(Multiname::any_attribute()),
)
.into())
}
Expand Down Expand Up @@ -733,7 +733,7 @@ pub fn append_child<'gc>(
let child = crate::avm2::e4x::maybe_escape_child(activation, child)?;

// 1. Let children be the result of calling the [[Get]] method of x with argument "*"
let name = Multiname::any(activation.gc());
let name = Multiname::any();
let children = xml.get_property_local(&name, activation)?;

// 2. Call the [[Put]] method of children with arguments children.[[Length]] and child
Expand Down Expand Up @@ -1102,7 +1102,7 @@ pub fn set_children<'gc>(
let value = args.get_value(0);

// 1. Call the [[Put]] method of x with arguments "*" and value
xml.set_property_local(&Multiname::any(activation.gc()), value, activation)?;
xml.set_property_local(&Multiname::any(), value, activation)?;

// 2. Return x
Ok(xml.into())
Expand Down
4 changes: 2 additions & 2 deletions core/src/avm2/globals/xml_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ pub fn children<'gc>(
activation,
sub_children,
Some(list.into()),
Some(Multiname::any(activation.gc())),
Some(Multiname::any()),
)
.into())
}
Expand Down Expand Up @@ -320,7 +320,7 @@ pub fn attributes<'gc>(
activation,
child_attrs,
Some(list.into()),
Some(Multiname::any_attribute(activation.gc())),
Some(Multiname::any_attribute()),
)
.into())
}
Expand Down
4 changes: 2 additions & 2 deletions core/src/avm2/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ impl<'gc> BytecodeMethod<'gc> {
) -> Result<Self, Error<'gc>> {
let abc = txunit.abc();
let mut signature = Vec::new();
let mut return_type = Multiname::any(activation.gc());
let mut return_type = Multiname::any();

if abc.methods.get(abc_method.0 as usize).is_some() {
let method = &abc.methods[abc_method.0 as usize];
Expand Down Expand Up @@ -429,7 +429,7 @@ impl<'gc> Method<'gc> {
signature: Vec::new(),
resolved_signature: GcCell::new(mc, None),
// FIXME - take in the real return type. This is needed for 'describeType'
return_type: Multiname::any(mc),
return_type: Multiname::any(),
is_variadic: true,
},
))
Expand Down
8 changes: 4 additions & 4 deletions core/src/avm2/multiname.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,19 +328,19 @@ impl<'gc> Multiname<'gc> {
}

/// Indicates the any type (any name in any namespace).
pub fn any(mc: &Mutation<'gc>) -> Self {
pub fn any() -> Self {
Self {
ns: NamespaceSet::single(Namespace::any(mc)),
ns: NamespaceSet::single(Namespace::any()),
name: None,
param: None,
flags: Default::default(),
}
}

/// Indicates the any attribute type (any attribute in any namespace).
pub fn any_attribute(mc: &Mutation<'gc>) -> Self {
pub fn any_attribute() -> Self {
Self {
ns: NamespaceSet::single(Namespace::any(mc)),
ns: NamespaceSet::single(Namespace::any()),
name: None,
param: None,
flags: MultinameFlags::ATTRIBUTE,
Expand Down
69 changes: 35 additions & 34 deletions core/src/avm2/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::avm2::Error;
use crate::context::UpdateContext;
use crate::string::{AvmAtom, AvmString};
use crate::{avm2::script::TranslationUnit, context::GcContext};
use gc_arena::{Collect, Gc, Mutation};
use gc_arena::{Collect, Gc};
use num_traits::FromPrimitive;
use ruffle_wstr::WStr;
use std::fmt::Debug;
Expand All @@ -12,7 +12,10 @@ use super::api_version::ApiVersion;

#[derive(Clone, Copy, Collect, Debug, PartialEq)]
#[collect(no_drop)]
pub struct Namespace<'gc>(Gc<'gc, NamespaceData<'gc>>);
pub struct Namespace<'gc>(
// `None` represents the wildcard namespace `Namespace::any()`.
Option<Gc<'gc, NamespaceData<'gc>>>,
);

/// Represents the name of a namespace.
#[allow(clippy::enum_variant_names)]
Expand All @@ -29,7 +32,6 @@ enum NamespaceData<'gc> {
// note: private namespaces are always compared by pointer identity
// of the enclosing `Gc`.
Private(AvmAtom<'gc>),
Any,
}

fn strip_version_mark(url: &WStr, is_playerglobals: bool) -> Option<(&WStr, ApiVersion)> {
Expand Down Expand Up @@ -85,7 +87,7 @@ impl<'gc> Namespace<'gc> {
context: &mut UpdateContext<'gc>,
) -> Result<Self, Error<'gc>> {
if namespace_index.0 == 0 {
return Ok(Self::any(context.gc_context));
return Ok(Self::any());
}

let actual_index = namespace_index.0 as usize - 1;
Expand All @@ -111,10 +113,10 @@ impl<'gc> Namespace<'gc> {

// Private namespaces don't get any of the namespace version checks
if let AbcNamespace::Private(_) = abc_namespace {
return Ok(Self(Gc::new(
return Ok(Self(Some(Gc::new(
context.gc_context,
NamespaceData::Private(namespace_name),
)));
))));
}

// FIXME - AvmCore gets this from an external source. I'm not exactly sure
Expand Down Expand Up @@ -212,11 +214,11 @@ impl<'gc> Namespace<'gc> {
AbcNamespace::StaticProtected(_) => NamespaceData::StaticProtected(namespace_name),
AbcNamespace::Private(_) => unreachable!(),
};
Ok(Self(Gc::new(context.gc_context, ns)))
Ok(Self(Some(Gc::new(context.gc_context, ns))))
}

pub fn any(mc: &Mutation<'gc>) -> Self {
Self(Gc::new(mc, NamespaceData::Any))
pub fn any() -> Self {
Self(None)
}

// TODO(moulins): allow passing an AvmAtom or a non-static `&WStr` directly
Expand All @@ -228,10 +230,10 @@ impl<'gc> Namespace<'gc> {
let atom = context
.interner
.intern(context.gc_context, package_name.into());
Self(Gc::new(
Self(Some(Gc::new(
context.gc_context,
NamespaceData::Namespace(atom, api_version),
))
)))
}

// TODO(moulins): allow passing an AvmAtom or a non-static `&WStr` directly
Expand All @@ -242,42 +244,41 @@ impl<'gc> Namespace<'gc> {
let atom = context
.interner
.intern(context.gc_context, package_name.into());
Self(Gc::new(
Self(Some(Gc::new(
context.gc_context,
NamespaceData::PackageInternal(atom),
))
)))
}

pub fn is_public(&self) -> bool {
matches!(*self.0, NamespaceData::Namespace(name, _) if name.as_wstr().is_empty())
matches!(self.0.as_deref(), Some(NamespaceData::Namespace(name, _)) if name.as_wstr().is_empty())
}

pub fn is_public_ignoring_ns(&self) -> bool {
matches!(*self.0, NamespaceData::Namespace(_, _))
matches!(self.0.as_deref(), Some(NamespaceData::Namespace(_, _)))
}

pub fn is_any(&self) -> bool {
matches!(*self.0, NamespaceData::Any)
self.0.is_none()
}

pub fn is_private(&self) -> bool {
matches!(*self.0, NamespaceData::Private(_))
matches!(self.0.as_deref(), Some(NamespaceData::Private(_)))
}

pub fn is_namespace(&self) -> bool {
matches!(*self.0, NamespaceData::Namespace(_, _))
matches!(self.0.as_deref(), Some(NamespaceData::Namespace(_, _)))
}

pub fn as_uri_opt(&self) -> Option<AvmString<'gc>> {
match *self.0 {
NamespaceData::Namespace(a, _) => Some(a.into()),
NamespaceData::PackageInternal(a) => Some(a.into()),
NamespaceData::Protected(a) => Some(a.into()),
NamespaceData::Explicit(a) => Some(a.into()),
NamespaceData::StaticProtected(a) => Some(a.into()),
NamespaceData::Private(a) => Some(a.into()),
NamespaceData::Any => None,
}
self.0.map(|data| match *data {
NamespaceData::Namespace(a, _) => a.into(),
NamespaceData::PackageInternal(a) => a.into(),
NamespaceData::Protected(a) => a.into(),
NamespaceData::Explicit(a) => a.into(),
NamespaceData::StaticProtected(a) => a.into(),
NamespaceData::Private(a) => a.into(),
})
}

/// Get the string value of this namespace, ignoring its type.
Expand All @@ -294,12 +295,12 @@ impl<'gc> Namespace<'gc> {
/// Namespace does not implement `PartialEq`, so that each caller is required
/// to explicitly choose either `exact_version_match` or `matches_ns`.
pub fn exact_version_match(&self, other: Self) -> bool {
if Gc::as_ptr(self.0) == Gc::as_ptr(other.0) {
if self.0.map(Gc::as_ptr) == other.0.map(Gc::as_ptr) {
true
} else if self.is_private() || other.is_private() {
false
} else {
*self.0 == *other.0
self.0 == other.0
}
}

Expand All @@ -312,10 +313,10 @@ impl<'gc> Namespace<'gc> {
if self.exact_version_match(other) {
return true;
}
match (&*self.0, &*other.0) {
match (self.0.as_deref(), other.0.as_deref()) {
(
NamespaceData::Namespace(name1, version1),
NamespaceData::Namespace(name2, version2),
Some(NamespaceData::Namespace(name1, version1)),
Some(NamespaceData::Namespace(name2, version2)),
) => {
let name_matches = name1 == name2;
let version_matches = version1 <= version2;
Expand All @@ -326,8 +327,8 @@ impl<'gc> Namespace<'gc> {
}
}
pub fn matches_api_version(&self, match_version: ApiVersion) -> bool {
match &*self.0 {
NamespaceData::Namespace(_, namespace_version) => namespace_version <= &match_version,
match self.0.as_deref() {
Some(NamespaceData::Namespace(_, version)) => version <= &match_version,
_ => true,
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/avm2/object/function_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub fn function_allocator<'gc>(
name: "<Empty Function>",
signature: vec![],
resolved_signature: GcCell::new(activation.context.gc_context, None),
return_type: Multiname::any(activation.context.gc_context),
return_type: Multiname::any(),
is_variadic: true,
},
);
Expand Down
2 changes: 1 addition & 1 deletion core/src/avm2/object/qname_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub fn q_name_allocator<'gc>(
activation.context.gc_context,
QNameObjectData {
base,
name: RefLock::new(Multiname::any(activation.context.gc_context)),
name: RefLock::new(Multiname::any()),
},
))
.into())
Expand Down
6 changes: 1 addition & 5 deletions core/src/avm2/object/xml_list_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -991,11 +991,7 @@ impl<'gc> TObject<'gc> for XmlListObject<'gc> {
// 2.h.i. Call the [[Put]] method of x[i] with arguments "*" and V
self.xml_object_child(index, activation)
.unwrap()
.set_property_local(
&Multiname::any(activation.gc()),
value,
activation,
)?;
.set_property_local(&Multiname::any(), value, activation)?;
}

// NOTE: Not specified in the spec, but avmplus returns here, so we do the same.
Expand Down
Loading
Loading