From d306162997675f90262b5e42f39b26fa590fdb0f Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Sat, 16 Mar 2024 11:09:42 +0200 Subject: [PATCH] Obsolete and cleanup some ObjectInstance access patterns (#1809) --- Jint/Engine.cs | 61 ++++++------------- Jint/Native/Array/ArrayInstance.cs | 13 +--- Jint/Native/JsArguments.cs | 2 +- Jint/Native/Object/ObjectInstance.cs | 33 +++------- .../Runtime/Environments/GlobalEnvironment.cs | 2 +- .../Runtime/Environments/ObjectEnvironment.cs | 11 ++-- .../Expressions/JintUnaryExpression.cs | 2 +- 7 files changed, 34 insertions(+), 90 deletions(-) diff --git a/Jint/Engine.cs b/Jint/Engine.cs index 98b6c80b50..d25c4a3bc3 100644 --- a/Jint/Engine.cs +++ b/Jint/Engine.cs @@ -597,56 +597,31 @@ internal JsValue GetValue(Reference reference, bool returnReferenceToPool) var v = baseObj.Get(property, reference.ThisValue); return v; } - else - { - // check if we are accessing a string, boxing operation can be costly to do index access - // we have good chance to have fast path with integer or string indexer - ObjectInstance? o = null; - if ((property._type & (InternalTypes.String | InternalTypes.Integer)) != InternalTypes.Empty - && baseValue is JsString s - && TryHandleStringValue(property, s, ref o, out var jsValue)) - { - return jsValue; - } - - if (o is null) - { - o = Runtime.TypeConverter.ToObject(Realm, baseValue); - } - - if (reference.IsPrivateReference) - { - return o.PrivateGet((PrivateName) reference.ReferencedName); - } - - var desc = o.GetProperty(property); - if (desc == PropertyDescriptor.Undefined) - { - return JsValue.Undefined; - } - if (desc.IsDataDescriptor()) - { - return desc.Value; - } + // check if we are accessing a string, boxing operation can be costly to do index access + // we have good chance to have fast path with integer or string indexer + ObjectInstance? o = null; + if ((property._type & (InternalTypes.String | InternalTypes.Integer)) != InternalTypes.Empty + && baseValue is JsString s + && TryHandleStringValue(property, s, ref o, out var jsValue)) + { + return jsValue; + } - var getter = desc.Get!; - if (getter.IsUndefined()) - { - return JsValue.Undefined; - } + if (o is null) + { + o = Runtime.TypeConverter.ToObject(Realm, baseValue); + } - var callable = (ICallable) getter; - return callable.Call(baseValue, Arguments.Empty); + if (reference.IsPrivateReference) + { + return o.PrivateGet((PrivateName) reference.ReferencedName); } - } - var record = baseValue as Environment; - if (record is null) - { - ExceptionHelper.ThrowArgumentException(); + return o.Get(property, reference.ThisValue); } + var record = (Environment) baseValue; var bindingValue = record.GetBindingValue(reference.ReferencedName.ToString(), reference.Strict); if (returnReferenceToPool) diff --git a/Jint/Native/Array/ArrayInstance.cs b/Jint/Native/Array/ArrayInstance.cs index 1087511fd2..8fb21e70f8 100644 --- a/Jint/Native/Array/ArrayInstance.cs +++ b/Jint/Native/Array/ArrayInstance.cs @@ -292,17 +292,6 @@ private bool DefineOwnProperty(uint index, PropertyDescriptor desc) [MethodImpl(MethodImplOptions.AggressiveInlining)] private JsNumber GetJsNumberLength() => _length is null ? JsNumber.PositiveZero : (JsNumber) _length._value!; - protected sealed override void AddProperty(JsValue property, PropertyDescriptor descriptor) - { - if (CommonProperties.Length.Equals(property )) - { - _length = descriptor; - return; - } - - base.AddProperty(property, descriptor); - } - protected sealed override bool TryGetProperty(JsValue property, [NotNullWhen(true)] out PropertyDescriptor? descriptor) { if (CommonProperties.Length.Equals(property)) @@ -436,7 +425,7 @@ internal JsValue Get(uint index) { if (!TryGetValue(index, out var value)) { - value = UnwrapJsValue(Prototype?.GetProperty(JsString.Create(index)) ?? PropertyDescriptor.Undefined); + value = Prototype?.Get(JsString.Create(index)) ?? Undefined; } return value; diff --git a/Jint/Native/JsArguments.cs b/Jint/Native/JsArguments.cs index b0374d79f2..d0f7514e80 100644 --- a/Jint/Native/JsArguments.cs +++ b/Jint/Native/JsArguments.cs @@ -153,7 +153,7 @@ public override bool Set(JsValue property, JsValue value, JsValue receiver) } // property is an accessor or inherited - var desc = GetProperty(property); + var desc = GetOwnProperty(property); if (desc.IsAccessorDescriptor()) { diff --git a/Jint/Native/Object/ObjectInstance.cs b/Jint/Native/Object/ObjectInstance.cs index 87511bc354..9ad75ca337 100644 --- a/Jint/Native/Object/ObjectInstance.cs +++ b/Jint/Native/Object/ObjectInstance.cs @@ -179,13 +179,6 @@ internal void SetProperty(Key property, PropertyDescriptor value) _properties[property] = value; } - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal void SetDataProperty(string property, JsValue value) - { - _properties ??= new PropertyDictionary(); - _properties[property] = new PropertyDescriptor(value, PropertyFlag.ConfigurableEnumerableWritable); - } - [MethodImpl(MethodImplOptions.NoInlining)] private void SetPropertyUnlikely(JsValue property, PropertyDescriptor value) { @@ -326,6 +319,7 @@ private List GetOwnPropertyKeysSorted(List initialOwnPropertyK internal virtual IEnumerable GetInitialOwnStringPropertyKeys() => System.Linq.Enumerable.Empty(); + [Obsolete("Will be removed")] protected virtual void AddProperty(JsValue property, PropertyDescriptor descriptor) { SetProperty(property, descriptor); @@ -453,9 +447,7 @@ protected internal virtual void SetOwnProperty(JsValue property, PropertyDescrip SetProperty(property, desc); } - /// - /// http://www.ecma-international.org/ecma-262/5.1/#sec-8.12.2 - /// + [Obsolete("Use Get or GetOwnProperty")] [MethodImpl(MethodImplOptions.AggressiveInlining)] public PropertyDescriptor GetProperty(JsValue property) { @@ -473,13 +465,8 @@ public bool TryGetValue(JsValue property, out JsValue value) { value = Undefined; var desc = GetOwnProperty(property); - if (desc != null && desc != PropertyDescriptor.Undefined) + if (desc != PropertyDescriptor.Undefined) { - if (desc == PropertyDescriptor.Undefined) - { - return false; - } - var descValue = desc.Value; if (desc.WritableSet && !ReferenceEquals(descValue, null)) { @@ -500,12 +487,7 @@ public bool TryGetValue(JsValue property, out JsValue value) return true; } - if (ReferenceEquals(Prototype, null)) - { - return false; - } - - return Prototype.TryGetValue(property, out value); + return Prototype?.TryGetValue(property, out value) == true; } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -654,7 +636,7 @@ internal bool CanPut(JsValue property) return Extensible; } - var inherited = Prototype.GetProperty(property); + var inherited = Prototype.GetOwnProperty(property); if (inherited == PropertyDescriptor.Undefined) { @@ -1345,7 +1327,7 @@ internal virtual bool CreateMethodProperty(JsValue p, JsValue v) } /// - /// https://tc39.es/ecma262/#sec-createdatapropertyorthrow + /// https://tc39.es/ecma262/#sec-createdataproperty /// [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool CreateDataProperty(JsValue p, JsValue v) @@ -1353,9 +1335,8 @@ public bool CreateDataProperty(JsValue p, JsValue v) return DefineOwnProperty(p, new PropertyDescriptor(v, PropertyFlag.ConfigurableEnumerableWritable)); } - /// - /// https://tc39.es/ecma262/#sec-createdataproperty + /// https://tc39.es/ecma262/#sec-createdatapropertyorthrow /// [MethodImpl(MethodImplOptions.AggressiveInlining)] internal bool CreateDataPropertyOrThrow(JsValue p, JsValue v) diff --git a/Jint/Runtime/Environments/GlobalEnvironment.cs b/Jint/Runtime/Environments/GlobalEnvironment.cs index 9c441bb9b7..00da01fb71 100644 --- a/Jint/Runtime/Environments/GlobalEnvironment.cs +++ b/Jint/Runtime/Environments/GlobalEnvironment.cs @@ -240,7 +240,7 @@ internal override JsValue GetBindingValue(Key name, bool strict) } else { - desc = _global.GetProperty(name.Name); + desc = _global.GetOwnProperty(name.Name); } if (strict && desc == PropertyDescriptor.Undefined) diff --git a/Jint/Runtime/Environments/ObjectEnvironment.cs b/Jint/Runtime/Environments/ObjectEnvironment.cs index 1838d7b134..b2e6c8304a 100644 --- a/Jint/Runtime/Environments/ObjectEnvironment.cs +++ b/Jint/Runtime/Environments/ObjectEnvironment.cs @@ -88,8 +88,7 @@ internal override bool TryGetBinding( return false; } - var desc = _bindingObject.GetProperty(name.Value); - value = ObjectInstance.UnwrapJsValue(desc, _bindingObject); + value = _bindingObject.Get(name.Value); return true; } @@ -154,13 +153,13 @@ internal override void SetMutableBinding(BindingName name, JsValue value, bool s internal override JsValue GetBindingValue(Key name, bool strict) { - var desc = _bindingObject.GetProperty(name.Name); - if (strict && desc == PropertyDescriptor.Undefined) + JsValue value; + if (!_bindingObject.TryGetValue(name.Name, out value) && strict) { - ExceptionHelper.ThrowReferenceNameError(_engine.Realm, name); + ExceptionHelper.ThrowReferenceNameError(_engine.Realm, name.Name); } - return ObjectInstance.UnwrapJsValue(desc, _bindingObject); + return value; } internal override bool DeleteBinding(Key name) => _bindingObject.Delete(name.Name); diff --git a/Jint/Runtime/Interpreter/Expressions/JintUnaryExpression.cs b/Jint/Runtime/Interpreter/Expressions/JintUnaryExpression.cs index e4daf2a883..bcbb78796a 100644 --- a/Jint/Runtime/Interpreter/Expressions/JintUnaryExpression.cs +++ b/Jint/Runtime/Interpreter/Expressions/JintUnaryExpression.cs @@ -223,7 +223,7 @@ private JsValue EvaluateJsValue(EvaluationContext context) ExceptionHelper.ThrowTypeError(engine.Realm, $"Cannot delete property '{referencedName}' of {o}"); } - if (StrictModeScope.IsStrictModeCode && !r.Base.AsObject().GetProperty(referencedName).Configurable) + if (StrictModeScope.IsStrictModeCode && !r.Base.AsObject().GetOwnProperty(referencedName).Configurable) { ExceptionHelper.ThrowTypeError(engine.Realm, $"Cannot delete property '{referencedName}' of {o}"); }