diff --git a/src/objects/js-objects.cc b/src/objects/js-objects.cc index 761b5ac9c0..3c27a202cb 100644 --- a/src/objects/js-objects.cc +++ b/src/objects/js-objects.cc @@ -1281,7 +1281,7 @@ Maybe JSReceiver::IsCompatiblePropertyDescriptor( isolate, nullptr, extensible, desc, current, should_throw, property_name); } -// ES6 9.1.6.3 +// https://tc39.es/ecma262/#sec-validateandapplypropertydescriptor // static Maybe JSReceiver::ValidateAndApplyPropertyDescriptor( Isolate* isolate, LookupIterator* it, bool extensible, @@ -1308,8 +1308,8 @@ Maybe JSReceiver::ValidateAndApplyPropertyDescriptor( } // 2c. If IsGenericDescriptor(Desc) or IsDataDescriptor(Desc) is true, then: // (This is equivalent to !IsAccessorDescriptor(desc).) - DCHECK((desc_is_generic_descriptor || desc_is_data_descriptor) == - !desc_is_accessor_descriptor); + DCHECK_EQ(desc_is_generic_descriptor || desc_is_data_descriptor, + !desc_is_accessor_descriptor); if (!desc_is_accessor_descriptor) { // 2c i. If O is not undefined, create an own data property named P of // object O whose [[Value]], [[Writable]], [[Enumerable]] and @@ -1356,10 +1356,8 @@ Maybe JSReceiver::ValidateAndApplyPropertyDescriptor( // 2e. Return true. return Just(true); } - // 3. Return true, if every field in Desc is absent. - // 4. Return true, if every field in Desc also occurs in current and the - // value of every field in Desc is the same value as the corresponding field - // in current when compared using the SameValue algorithm. + // 3. If every field in Desc is absent, return true. (This also has a shortcut + // not in the spec: if every field value matches the current value, return.) if ((!desc->has_enumerable() || desc->enumerable() == current->enumerable()) && (!desc->has_configurable() || @@ -1374,18 +1372,19 @@ Maybe JSReceiver::ValidateAndApplyPropertyDescriptor( (current->has_set() && current->set()->SameValue(*desc->set())))) { return Just(true); } - // 5. If the [[Configurable]] field of current is false, then + // 4. If current.[[Configurable]] is false, then if (!current->configurable()) { - // 5a. Return false, if the [[Configurable]] field of Desc is true. + // 4a. If Desc.[[Configurable]] is present and its value is true, return + // false. if (desc->has_configurable() && desc->configurable()) { RETURN_FAILURE( isolate, GetShouldThrow(isolate, should_throw), NewTypeError(MessageTemplate::kRedefineDisallowed, it != nullptr ? it->GetName() : property_name)); } - // 5b. Return false, if the [[Enumerable]] field of Desc is present and the - // [[Enumerable]] fields of current and Desc are the Boolean negation of - // each other. + // 4b. If Desc.[[Enumerable]] is present and + // ! SameValue(Desc.[[Enumerable]], current.[[Enumerable]]) is false, return + // false. if (desc->has_enumerable() && desc->enumerable() != current->enumerable()) { RETURN_FAILURE( isolate, GetShouldThrow(isolate, should_throw), @@ -1396,79 +1395,79 @@ Maybe JSReceiver::ValidateAndApplyPropertyDescriptor( bool current_is_data_descriptor = PropertyDescriptor::IsDataDescriptor(current); - // 6. If IsGenericDescriptor(Desc) is true, no further validation is required. + // 5. If ! IsGenericDescriptor(Desc) is true, no further validation is + // required. if (desc_is_generic_descriptor) { // Nothing to see here. - // 7. Else if IsDataDescriptor(current) and IsDataDescriptor(Desc) have - // different results, then: + // 6. Else if ! SameValue(!IsDataDescriptor(current), + // !IsDataDescriptor(Desc)) is false, the } else if (current_is_data_descriptor != desc_is_data_descriptor) { - // 7a. Return false, if the [[Configurable]] field of current is false. + // 6a. If current.[[Configurable]] is false, return false. if (!current->configurable()) { RETURN_FAILURE( isolate, GetShouldThrow(isolate, should_throw), NewTypeError(MessageTemplate::kRedefineDisallowed, it != nullptr ? it->GetName() : property_name)); } - // 7b. If IsDataDescriptor(current) is true, then: + // 6b. If IsDataDescriptor(current) is true, then: if (current_is_data_descriptor) { - // 7b i. If O is not undefined, convert the property named P of object O + // 6b i. If O is not undefined, convert the property named P of object O // from a data property to an accessor property. Preserve the existing // values of the converted property's [[Configurable]] and [[Enumerable]] // attributes and set the rest of the property's attributes to their // default values. - // --> Folded into step 10. + // --> Folded into step 9 } else { - // 7c i. If O is not undefined, convert the property named P of object O + // 6c i. If O is not undefined, convert the property named P of object O // from an accessor property to a data property. Preserve the existing // values of the converted property’s [[Configurable]] and [[Enumerable]] // attributes and set the rest of the property’s attributes to their // default values. - // --> Folded into step 10. + // --> Folded into step 9 } - // 8. Else if IsDataDescriptor(current) and IsDataDescriptor(Desc) are both + // 7. Else if IsDataDescriptor(current) and IsDataDescriptor(Desc) are both // true, then: } else if (current_is_data_descriptor && desc_is_data_descriptor) { - // 8a. If the [[Configurable]] field of current is false, then: - if (!current->configurable()) { - // 8a i. Return false, if the [[Writable]] field of current is false and - // the [[Writable]] field of Desc is true. - if (!current->writable() && desc->has_writable() && desc->writable()) { + // 7a. If current.[[Configurable]] is false and current.[[Writable]] is + // false, then + if (!current->configurable() && !current->writable()) { + // 7a i. If Desc.[[Writable]] is present and Desc.[[Writable]] is true, + // return false. + if (desc->has_writable() && desc->writable()) { RETURN_FAILURE( isolate, GetShouldThrow(isolate, should_throw), NewTypeError(MessageTemplate::kRedefineDisallowed, it != nullptr ? it->GetName() : property_name)); } - // 8a ii. If the [[Writable]] field of current is false, then: - if (!current->writable()) { - // 8a ii 1. Return false, if the [[Value]] field of Desc is present and - // SameValue(Desc.[[Value]], current.[[Value]]) is false. - if (desc->has_value() && !desc->value()->SameValue(*current->value())) { - RETURN_FAILURE( - isolate, GetShouldThrow(isolate, should_throw), - NewTypeError(MessageTemplate::kRedefineDisallowed, - it != nullptr ? it->GetName() : property_name)); - } + // 7a ii. If Desc.[[Value]] is present and SameValue(Desc.[[Value]], + // current.[[Value]]) is false, return false. + if (desc->has_value() && !desc->value()->SameValue(*current->value())) { + RETURN_FAILURE( + isolate, GetShouldThrow(isolate, should_throw), + NewTypeError(MessageTemplate::kRedefineDisallowed, + it != nullptr ? it->GetName() : property_name)); } } } else { - // 9. Else IsAccessorDescriptor(current) and IsAccessorDescriptor(Desc) - // are both true, + // 8. Else, + // 8a. Assert: ! IsAccessorDescriptor(current) and + // ! IsAccessorDescriptor(Desc) are both true. DCHECK(PropertyDescriptor::IsAccessorDescriptor(current) && desc_is_accessor_descriptor); - // 9a. If the [[Configurable]] field of current is false, then: + // 8b. If current.[[Configurable]] is false, then: if (!current->configurable()) { - // 9a i. Return false, if the [[Set]] field of Desc is present and - // SameValue(Desc.[[Set]], current.[[Set]]) is false. + // 8a i. If Desc.[[Set]] is present and SameValue(Desc.[[Set]], + // current.[[Set]]) is false, return false. if (desc->has_set() && !desc->set()->SameValue(*current->set())) { RETURN_FAILURE( isolate, GetShouldThrow(isolate, should_throw), NewTypeError(MessageTemplate::kRedefineDisallowed, it != nullptr ? it->GetName() : property_name)); } - // 9a ii. Return false, if the [[Get]] field of Desc is present and - // SameValue(Desc.[[Get]], current.[[Get]]) is false. + // 8a ii. If Desc.[[Get]] is present and SameValue(Desc.[[Get]], + // current.[[Get]]) is false, return false. if (desc->has_get() && !desc->get()->SameValue(*current->get())) { RETURN_FAILURE( isolate, GetShouldThrow(isolate, should_throw), @@ -1478,9 +1477,9 @@ Maybe JSReceiver::ValidateAndApplyPropertyDescriptor( } } - // 10. If O is not undefined, then: + // 9. If O is not undefined, then: if (it != nullptr) { - // 10a. For each field of Desc that is present, set the corresponding + // 9a. For each field of Desc that is present, set the corresponding // attribute of the property named P of object O to the value of the field. PropertyAttributes attrs = NONE; @@ -1537,7 +1536,7 @@ Maybe JSReceiver::ValidateAndApplyPropertyDescriptor( } } - // 11. Return true. + // 10. Return true. return Just(true); }