Internalize Environment services and use Key as name (sebastienros#1735)

* make Environment and GlobalEnvironment internal, use Key as lookup

Jint.Tests.PublicInterface/RavenApiUsageTests.cs

@@ -34,9 +34,7 @@ public void CanBuildCustomScriptFunctionInstance()
         var functionObject = new ScriptFunction(
             engine,
             functionExp,
-            engine.Advanced.CreateDeclarativeEnvironment(),
-            strict: false
-        );
+            strict: false);
 
         Assert.NotNull(functionObject);
     }

Jint.Tests/Runtime/InteropTests.cs

@@ -3,7 +3,6 @@
 using System.Reflection;
 using System.Runtime.CompilerServices;
 using Jint.Native;
-using Jint.Native.Object;
 using Jint.Native.Symbol;
 using Jint.Runtime;
 using Jint.Runtime.Interop;

Jint/Engine.Advanced.cs

@@ -1,6 +1,4 @@
 using Jint.Native.Promise;
-using Jint.Runtime.Environments;
-using Environment = Jint.Runtime.Environments.Environment;
 
 namespace Jint;
 
@@ -51,14 +49,6 @@ public void ProcessTasks()
         _engine.RunAvailableContinuations();
     }
 
-    /// <summary>
-    /// Creates a new declarative environment that has current lexical environment as outer scope.
-    /// </summary>
-    public Environment CreateDeclarativeEnvironment()
-    {
-        return JintEnvironment.NewDeclarativeEnvironment(_engine, _engine.ExecutionContext.LexicalEnvironment);
-    }
-
     /// <summary>
     /// EXPERIMENTAL! Subject to change.
     ///

Jint/Engine.Ast.cs

@@ -1,3 +1,4 @@
+using System.Runtime.InteropServices;
 using Esprima;
 using Esprima.Ast;
 using Jint.Native;
@@ -126,7 +127,8 @@ internal static void GatherLexNames(HoistingScope scope, List<CachedLexicalName>
         }
     }
 
-    internal readonly record struct CachedLexicalName(string Name, bool Constant);
+    [StructLayout(LayoutKind.Auto)]
+    internal readonly record struct CachedLexicalName(Key Name, bool Constant);
 
     public HoistingScope Scope { get; }
     public List<string> VarNames { get; }

Jint/Engine.cs

@@ -942,10 +942,9 @@ private void GlobalDeclarationInstantiation(
         {
             var hoistingScope = script.GetHoistingScope();
             var functionDeclarations = hoistingScope._functionDeclarations;
-            var lexDeclarations = hoistingScope._lexicalDeclarations;
 
             var functionToInitialize = new List<JintFunctionDefinition>();
-            var declaredFunctionNames = new HashSet<string>(StringComparer.Ordinal);
+            var declaredFunctionNames = new HashSet<Key>();
             var declaredVarNames = new List<string>();
 
             var realm = Realm;
@@ -955,7 +954,7 @@ private void GlobalDeclarationInstantiation(
                 for (var i = functionDeclarations.Count - 1; i >= 0; i--)
                 {
                     var d = functionDeclarations[i];
-                    var fn = d.Id!.Name;
+                    var fn = (Key) d.Id!.Name;
                     if (!declaredFunctionNames.Contains(fn))
                     {
                         var fnDefinable = env.CanDeclareGlobalFunction(fn);
@@ -973,7 +972,7 @@ private void GlobalDeclarationInstantiation(
             var varNames = script.GetVarNames(hoistingScope);
             for (var j = 0; j < varNames.Count; j++)
             {
-                var vn = varNames[j];
+                Key vn = varNames[j];
                 if (env.HasLexicalDeclaration(vn))
                 {
                     ExceptionHelper.ThrowSyntaxError(realm, $"Identifier '{vn}' has already been declared");
@@ -1144,7 +1143,7 @@ private void GlobalDeclarationInstantiation(
                 {
                     for (var j = 0; j < d.BoundNames.Count; j++)
                     {
-                        var dn = d.BoundNames[j];
+                        Key dn = d.BoundNames[j];
                         if (d.IsConstantDeclaration)
                         {
                             lexEnv.CreateImmutableBinding(dn, strict: true);
@@ -1260,14 +1259,14 @@ internal void EvalDeclarationInstantiation(
 
             var functionDeclarations = hoistingScope._functionDeclarations;
             var functionsToInitialize = new LinkedList<JintFunctionDefinition>();
-            var declaredFunctionNames = new HashSet<string>(StringComparer.Ordinal);
+            var declaredFunctionNames = new HashSet<Key>();
 
             if (functionDeclarations != null)
             {
                 for (var i = functionDeclarations.Count - 1; i >= 0; i--)
                 {
                     var d = functionDeclarations[i];
-                    var fn = d.Id!.Name;
+                    Key fn = d.Id!.Name;
                     if (!declaredFunctionNames.Contains(fn))
                     {
                         if (varEnvRec is GlobalEnvironment ger)
@@ -1286,7 +1285,7 @@ internal void EvalDeclarationInstantiation(
             }
 
             var boundNames = new List<string>();
-            var declaredVarNames = new List<string>();
+            var declaredVarNames = new List<Key>();
             var variableDeclarations = hoistingScope._variablesDeclarations;
             var variableDeclarationsCount = variableDeclarations?.Count;
             for (var i = 0; i < variableDeclarationsCount; i++)
@@ -1322,7 +1321,7 @@ internal void EvalDeclarationInstantiation(
                 d.GetBoundNames(boundNames);
                 for (var j = 0; j < boundNames.Count; j++)
                 {
-                    var dn = boundNames[j];
+                    Key dn = boundNames[j];
                     if (d.IsConstantDeclaration())
                     {
                         lexEnvRec.CreateImmutableBinding(dn, strict: true);
@@ -1336,7 +1335,7 @@ internal void EvalDeclarationInstantiation(
 
             foreach (var f in functionsToInitialize)
             {
-                var fn = f.Name!;
+                Key fn = f.Name!;
                 var fo = realm.Intrinsics.Function.InstantiateFunctionObject(f, lexEnv, privateEnv);
                 if (varEnvRec is GlobalEnvironment ger)
                 {

Jint/Key.cs

@@ -10,6 +10,7 @@ namespace Jint
     [DebuggerDisplay("{" + nameof(Name) + "}")]
     internal readonly struct Key : IEquatable<Key>
     {
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private Key(string name)
         {
             Name = name;
@@ -19,11 +20,10 @@ private Key(string name)
         internal readonly string Name;
         internal readonly int HashCode;
 
-        public static implicit operator Key(string name)
-        {
-            return new Key(name);
-        }
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static implicit operator Key(string name) => new(name);
 
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static implicit operator string(Key key) => key.Name;
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]

Jint/Native/Argument/JsArguments.cs

@@ -15,7 +15,7 @@ namespace Jint.Native.Argument
     public sealed class JsArguments : ObjectInstance
     {
         // cache property container for array iteration for less allocations
-        private static readonly ThreadLocal<HashSet<string>> _mappedNamed = new(() => new HashSet<string>(StringComparer.Ordinal));
+        private static readonly ThreadLocal<HashSet<Key>> _mappedNamed = new(() => []);
 
         private Function.Function _func = null!;
         private Key[] _names = null!;

Jint/Native/Function/Function.cs

@@ -21,7 +21,7 @@ public abstract partial class Function : ObjectInstance, ICallable
         protected internal PropertyDescriptor? _length;
         internal PropertyDescriptor? _nameDescriptor;
 
-        protected internal Environment? _environment;
+        internal Environment? _environment;
         internal readonly JintFunctionDefinition _functionDefinition = null!;
         internal readonly FunctionThisMode _thisMode;
         internal JsValue _homeObject = Undefined;

Jint/Native/Function/ScriptFunction.cs

@@ -23,13 +23,12 @@ public sealed class ScriptFunction : Function, IConstructor
         public ScriptFunction(
             Engine engine,
             IFunction functionDeclaration,
-            Environment env,
             bool strict,
             ObjectInstance? proto = null)
             : this(
                 engine,
                 new JintFunctionDefinition(functionDeclaration),
-                env,
+                JintEnvironment.NewDeclarativeEnvironment(engine, engine.ExecutionContext.LexicalEnvironment),
                 strict ? FunctionThisMode.Strict : FunctionThisMode.Global,
                 proto)
         {

Jint/Native/Global/GlobalObject.cs

@@ -673,19 +673,7 @@ internal bool HasProperty(Key property)
             return GetOwnProperty(property) != PropertyDescriptor.Undefined;
         }
 
-        internal PropertyDescriptor GetProperty(Key property) => GetOwnProperty(property);
-
-        internal bool DefinePropertyOrThrow(Key property, PropertyDescriptor desc)
-        {
-            if (!DefineOwnProperty(property, desc))
-            {
-                ExceptionHelper.ThrowTypeError(_realm);
-            }
-
-            return true;
-        }
-
-        internal bool DefineOwnProperty(Key property, PropertyDescriptor desc)
+        private bool DefineOwnProperty(Key property, PropertyDescriptor desc)
         {
             var current = GetOwnProperty(property);
             if (current == desc)
@@ -752,11 +740,5 @@ internal bool SetFromMutableBinding(Key property, JsValue value, bool strict)
 
             return true;
         }
-
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        internal void SetOwnProperty(Key property, PropertyDescriptor desc)
-        {
-            SetProperty(property, desc);
-        }
     }
 }

Jint/Native/Promise/JsPromise.cs

@@ -1,4 +1,3 @@
-using Jint.Native.Function;
 using Jint.Native.Object;
 using Jint.Runtime;
 using Jint.Runtime.Descriptors;

Jint/Native/ShadowRealm/ShadowRealm.cs

@@ -1,7 +1,6 @@
 using Esprima;
 using Esprima.Ast;
 using Esprima.Utils;
-using Jint.Native.Function;
 using Jint.Native.Object;
 using Jint.Native.Promise;
 using Jint.Runtime;

Jint/Runtime/Descriptors/Specialized/ClrAccessDescriptor.cs

@@ -38,7 +38,7 @@ private JsValue DoGet(JsValue n)
 
         private void DoSet(JsValue n, JsValue o)
         {
-            _env.SetMutableBinding(_name.Key.Name, o, true);
+            _env.SetMutableBinding(_name.Key, o, true);
         }
     }
 }

Jint/Runtime/Environments/DeclarativeEnvironment.cs

@@ -19,15 +19,9 @@ public DeclarativeEnvironment(Engine engine, bool catchEnvironment = false) : ba
             _catchEnvironment = catchEnvironment;
         }
 
-        public sealed override bool HasBinding(string name)
-        {
-            return _dictionary is not null && _dictionary.ContainsKey(name);
-        }
+        internal sealed override bool HasBinding(BindingName name) => HasBinding(name.Key);
 
-        internal sealed override bool HasBinding(BindingName name)
-        {
-            return _dictionary is not null &&_dictionary.ContainsKey(name.Key);
-        }
+        internal sealed override bool HasBinding(Key name) => _dictionary is not null && _dictionary.ContainsKey(name);
 
         internal override bool TryGetBinding(
             BindingName name,
@@ -53,39 +47,35 @@ internal void CreateImmutableBindingAndInitialize(Key name, bool strict, JsValue
             _dictionary[name] = new Binding(value, canBeDeleted: false, mutable: false, strict);
         }
 
-        public sealed override void CreateMutableBinding(string name, bool canBeDeleted = false)
+        internal sealed override void CreateMutableBinding(Key name, bool canBeDeleted = false)
         {
             _dictionary ??= new HybridDictionary<Binding>();
             _dictionary[name] = new Binding(null!, canBeDeleted, mutable: true, strict: false);
         }
 
-        public sealed override void CreateImmutableBinding(string name, bool strict = true)
+        internal sealed override void CreateImmutableBinding(Key name, bool strict = true)
         {
             _dictionary ??= new HybridDictionary<Binding>();
             _dictionary[name] = new Binding(null!, canBeDeleted: false, mutable: false, strict);
         }
 
-        public sealed override void InitializeBinding(string name, JsValue value)
+        internal sealed override void InitializeBinding(Key name, JsValue value)
         {
             _dictionary ??= new HybridDictionary<Binding>();
             _dictionary.SetOrUpdateValue(name, static (current, value) => current.ChangeValue(value), value);
         }
 
-        internal sealed override void SetMutableBinding(BindingName name, JsValue value, bool strict)
-        {
-            SetMutableBinding(name.Key.Name, value, strict);
-        }
+        internal sealed override void SetMutableBinding(BindingName name, JsValue value, bool strict) => SetMutableBinding(name.Key, value, strict);
 
-        public sealed override void SetMutableBinding(string name, JsValue value, bool strict)
+        internal sealed override void SetMutableBinding(Key name, JsValue value, bool strict)
         {
-            var key = (Key) name;
-            if (_dictionary is null || !_dictionary.TryGetValue(key, out var binding))
+            if (_dictionary is null || !_dictionary.TryGetValue(name, out var binding))
             {
                 if (strict)
                 {
                     ExceptionHelper.ThrowReferenceNameError(_engine.Realm, name);
                 }
-                CreateMutableBindingAndInitialize(key, canBeDeleted: true, value);
+                CreateMutableBindingAndInitialize(name, canBeDeleted: true, value);
                 return;
             }
 
@@ -102,7 +92,7 @@ public sealed override void SetMutableBinding(string name, JsValue value, bool s
 
             if (binding.Mutable)
             {
-                _dictionary[key] = binding.ChangeValue(value);
+                _dictionary[name] = binding.ChangeValue(value);
             }
             else
             {
@@ -113,7 +103,7 @@ public sealed override void SetMutableBinding(string name, JsValue value, bool s
             }
         }
 
-        public override JsValue GetBindingValue(string name, bool strict)
+        internal override JsValue GetBindingValue(Key name, bool strict)
         {
             if (_dictionary is not null && _dictionary.TryGetValue(name, out var binding) && binding.IsInitialized())
             {
@@ -130,7 +120,7 @@ private void ThrowUninitializedBindingError(string name)
             ExceptionHelper.ThrowReferenceError(_engine.Realm, $"Cannot access '{name}' before initialization");
         }
 
-        public sealed override bool DeleteBinding(string name)
+        internal sealed override bool DeleteBinding(Key name)
         {
             if (_dictionary is null || !_dictionary.TryGetValue(name, out var binding))
             {
@@ -147,13 +137,13 @@ public sealed override bool DeleteBinding(string name)
             return true;
         }
 
-        public override bool HasThisBinding() => false;
+        internal override bool HasThisBinding() => false;
 
-        public override bool HasSuperBinding() => false;
+        internal override bool HasSuperBinding() => false;
 
-        public sealed override JsValue WithBaseObject() => Undefined;
+        internal sealed override JsValue WithBaseObject() => Undefined;
 
-        public sealed override bool HasBindings()
+        internal sealed override bool HasBindings()
         {
             return _dictionary?.Count > 0;
         }
@@ -176,7 +166,7 @@ internal sealed override string[] GetAllBindingNames()
             return keys;
         }
 
-        public override JsValue GetThisBinding()
+        internal override JsValue GetThisBinding()
         {
             return Undefined;
         }