Skip to content

Commit

Permalink
Refactor Property Descriptor flags (#553)
Browse files Browse the repository at this point in the history
  • Loading branch information
@HalidOdat
HalidOdat committed Jul 21, 2020
1 parent d8eb7ca commit 5d4d8fe
Show file tree
Hide file tree
Showing 13 changed files with 570 additions and 195 deletions.
2 changes: 1 addition & 1 deletion .vscode/launch.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,4 @@
"externalConsole": true
},
]
}
}
30 changes: 20 additions & 10 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 9 additions & 12 deletions boa/src/builtins/array/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use super::function::{make_builtin_fn, make_constructor_fn};
use crate::{
builtins::{
object::{ObjectData, PROTOTYPE},
property::Property,
property::{Attribute, Property},
value::{same_value_zero, ResultValue, Value},
},
exec::Interpreter,
Expand Down Expand Up @@ -75,12 +75,10 @@ impl Array {
}

// Create length
let length = Property::new()
.value(Value::from(array_contents.len() as i32))
.writable(true)
.configurable(false)
.enumerable(false);

let length = Property::data_descriptor(
array_contents.len().into(),
Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
);
array_obj_ptr.set_property("length".to_string(), length);

for (n, value) in array_contents.iter().enumerate() {
Expand Down Expand Up @@ -143,11 +141,10 @@ impl Array {
}

// finally create length property
let length = Property::new()
.value(Value::from(length))
.writable(true)
.configurable(false)
.enumerable(false);
let length = Property::data_descriptor(
length.into(),
Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
);

this.set_property("length".to_string(), length);

Expand Down
38 changes: 18 additions & 20 deletions boa/src/builtins/function/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@

use crate::{
builtins::{
array::Array,
object::{Object, ObjectData, PROTOTYPE},
property::Property,
property::{Attribute, Property},
value::{RcString, ResultValue, Value},
Array,
},
environment::function_environment_record::BindingStatus,
environment::lexical_environment::{new_function_environment, Environment},
Expand Down Expand Up @@ -411,19 +411,19 @@ pub fn create_unmapped_arguments_object(arguments_list: &[Value]) -> Value {
let mut obj = Object::default();
obj.set_internal_slot("ParameterMap", Value::undefined());
// Set length
let mut length = Property::default();
length = length.writable(true).value(Value::from(len));
let length = Property::data_descriptor(
len.into(),
Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
);
// Define length as a property
obj.define_own_property("length".to_string(), length);
let mut index: usize = 0;
while index < len {
let val = arguments_list.get(index).expect("Could not get argument");
let mut prop = Property::default();
prop = prop
.value(val.clone())
.enumerable(true)
.writable(true)
.configurable(true);
let prop = Property::data_descriptor(
val.clone(),
Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE,
);

obj.properties_mut()
.insert(RcString::from(index.to_string()), prop);
Expand Down Expand Up @@ -469,18 +469,16 @@ pub fn make_constructor_fn(
let mut constructor =
Object::function(function, global.get_field("Function").get_field(PROTOTYPE));

let length = Property::new()
.value(Value::from(length))
.writable(false)
.configurable(false)
.enumerable(false);
let length = Property::data_descriptor(
length.into(),
Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
);
constructor.insert_property("length", length);

let name = Property::new()
.value(Value::from(name))
.writable(false)
.configurable(false)
.enumerable(false);
let name = Property::data_descriptor(
name.into(),
Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
);
constructor.insert_property("name", name);

let constructor = Value::from(constructor);
Expand Down
11 changes: 5 additions & 6 deletions boa/src/builtins/map/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use super::function::{make_builtin_fn, make_constructor_fn};
use crate::{
builtins::{
object::{ObjectData, PROTOTYPE},
property::Property,
property::{Attribute, Property},
value::{ResultValue, Value},
},
exec::Interpreter,
Expand All @@ -26,11 +26,10 @@ impl Map {

/// Helper function to set the size property.
fn set_size(this: &Value, size: usize) {
let size = Property::new()
.value(Value::from(size))
.writable(false)
.configurable(false)
.enumerable(false);
let size = Property::data_descriptor(
size.into(),
Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
);

this.set_property("size".to_string(), size);
}
Expand Down
87 changes: 35 additions & 52 deletions boa/src/builtins/object/internal_methods.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

use crate::builtins::{
object::{Object, PROTOTYPE},
property::Property,
property::{Attribute, Property},
value::{same_value, RcString, Value},
};
use crate::BoaProfiler;
Expand Down Expand Up @@ -73,7 +73,7 @@ impl Object {
{
return true;
}
if desc.configurable.expect("unable to get value") {
if desc.configurable_or(false) {
self.remove_property(&prop_key.to_string());
return true;
}
Expand Down Expand Up @@ -131,14 +131,14 @@ impl Object {
if !parent.is_null() {
// TODO: come back to this
}
own_desc = Property::new()
.writable(true)
.enumerable(true)
.configurable(true);
own_desc = Property::data_descriptor(
Value::undefined(),
Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE,
);
}
// [3]
if own_desc.is_data_descriptor() {
if !own_desc.writable.unwrap() {
if !own_desc.writable() {
return false;
}

Expand Down Expand Up @@ -184,18 +184,12 @@ impl Object {
}

// 4
if !current.configurable.unwrap_or(false) {
if desc.configurable.is_some() && desc.configurable.expect("unable to get prop desc") {
if !current.configurable_or(false) {
if desc.configurable_or(false) {
return false;
}

if desc.enumerable.is_some()
&& (desc.enumerable.as_ref().expect("unable to get prop desc")
!= current
.enumerable
.as_ref()
.expect("unable to get prop desc"))
{
if desc.enumerable_or(false) != current.enumerable_or(false) {
return false;
}
}
Expand All @@ -205,14 +199,14 @@ impl Object {
// 6
} else if current.is_data_descriptor() != desc.is_data_descriptor() {
// a
if !current.configurable.expect("unable to get prop desc") {
if !current.configurable() {
return false;
}
// b
if current.is_data_descriptor() {
// Convert to accessor
current.value = None;
current.writable = None;
current.attribute.remove(Attribute::WRITABLE);
} else {
// c
// convert to data
Expand All @@ -224,10 +218,8 @@ impl Object {
// 7
} else if current.is_data_descriptor() && desc.is_data_descriptor() {
// a
if !current.configurable.expect("unable to get prop desc")
&& !current.writable.expect("unable to get prop desc")
{
if desc.writable.is_some() && desc.writable.expect("unable to get prop desc") {
if !current.configurable() && !current.writable() {
if desc.writable_or(false) {
return false;
}

Expand All @@ -244,7 +236,7 @@ impl Object {
}
// 8
} else {
if !current.configurable.unwrap() {
if !current.configurable() {
if desc.set.is_some()
&& !same_value(&desc.set.clone().unwrap(), &current.set.clone().unwrap())
{
Expand Down Expand Up @@ -279,43 +271,35 @@ impl Object {
debug_assert!(Property::is_property_key(prop));
// Prop could either be a String or Symbol
match *prop {
Value::String(ref st) => {
self.properties()
.get(st)
.map_or_else(Property::default, |v| {
let mut d = Property::default();
if v.is_data_descriptor() {
d.value = v.value.clone();
d.writable = v.writable;
} else {
debug_assert!(v.is_accessor_descriptor());
d.get = v.get.clone();
d.set = v.set.clone();
}
d.enumerable = v.enumerable;
d.configurable = v.configurable;
d
})
}
Value::String(ref st) => self.properties().get(st).map_or_else(Property::empty, |v| {
let mut d = Property::empty();
if v.is_data_descriptor() {
d.value = v.value.clone();
} else {
debug_assert!(v.is_accessor_descriptor());
d.get = v.get.clone();
d.set = v.set.clone();
}
d.attribute = v.attribute;
d
}),
Value::Symbol(ref symbol) => {
self.symbol_properties()
.get(&symbol.hash())
.map_or_else(Property::default, |v| {
let mut d = Property::default();
.map_or_else(Property::empty, |v| {
let mut d = Property::empty();
if v.is_data_descriptor() {
d.value = v.value.clone();
d.writable = v.writable;
} else {
debug_assert!(v.is_accessor_descriptor());
d.get = v.get.clone();
d.set = v.set.clone();
}
d.enumerable = v.enumerable;
d.configurable = v.configurable;
d.attribute = v.attribute;
d
})
}
_ => Property::default(),
_ => unreachable!("the field can only be of type string or symbol"),
}
}

Expand Down Expand Up @@ -409,11 +393,10 @@ impl Object {
{
self.properties.insert(
name.into(),
Property::default()
.value(value)
.writable(true)
.configurable(true)
.enumerable(true),
Property::data_descriptor(
value,
Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE,
),
)
}

Expand Down
Loading

0 comments on commit 5d4d8fe

Please sign in to comment.