Ensure to use same JintFunctionDefinition instance when possible (#964)

* Ensure we use same JintFunctionDefinition when possible
* Add JintFunctionDefinition.ThisMode to remove repetition

Jint/Engine.cs

@@ -739,7 +739,7 @@ namespace Jint
             var varDeclarations = hoistingScope._variablesDeclarations;
             var lexDeclarations = hoistingScope._lexicalDeclarations;
 
-            var functionToInitialize = new LinkedList<FunctionDeclaration>();
+            var functionToInitialize = new LinkedList<JintFunctionDefinition>();
             var declaredFunctionNames = new HashSet<string>();
             var declaredVarNames = new List<string>();
 
@@ -760,7 +760,7 @@ namespace Jint
                         }
 
                         declaredFunctionNames.Add(fn);
-                        functionToInitialize.AddFirst(d);
+                        functionToInitialize.AddFirst(new JintFunctionDefinition(this, d));
                     }
                 }
             }
@@ -827,7 +827,7 @@ namespace Jint
 
             foreach (var f in functionToInitialize)
             {
-                var fn = f.Id!.Name;
+                var fn = f.Function.Id!.Name;
 
                 if (env.HasLexicalDeclaration(fn))
                 {
@@ -879,8 +879,7 @@ namespace Jint
                 {
                     // NOTE: mapped argument object is only provided for non-strict functions that don't have a rest parameter,
                     // any parameter default value initializers, or any destructured parameters.
-                    ao = CreateMappedArgumentsObject(functionInstance, parameterNames, argumentsList, envRec,
-                        configuration.HasRestParameter);
+                    ao = CreateMappedArgumentsObject(functionInstance, parameterNames, argumentsList, envRec, configuration.HasRestParameter);
                 }
 
                 if (strict)
@@ -971,14 +970,14 @@ namespace Jint
         }
 
         private void InitializeFunctions(
-            LinkedList<FunctionDeclaration> functionsToInitialize,
+            LinkedList<JintFunctionDefinition> functionsToInitialize,
             EnvironmentRecord lexEnv,
             DeclarativeEnvironmentRecord varEnvRec)
         {
             var realm = Realm;
             foreach (var f in functionsToInitialize)
             {
-                var fn = f.Id.Name;
+                var fn = f.Function.Id.Name;
                 var fo = realm.Intrinsics.Function.InstantiateFunctionObject(f, lexEnv);
                 varEnvRec.SetMutableBinding(fn, fo, strict: false);
             }
@@ -1076,7 +1075,7 @@ namespace Jint
             }
 
             var functionDeclarations = hoistingScope._functionDeclarations;
-            var functionsToInitialize = new LinkedList<FunctionDeclaration>();
+            var functionsToInitialize = new LinkedList<JintFunctionDefinition>();
             var declaredFunctionNames = new HashSet<string>();
 
             if (functionDeclarations != null)
@@ -1097,7 +1096,7 @@ namespace Jint
                         }
 
                         declaredFunctionNames.Add(fn);
-                        functionsToInitialize.AddFirst(d);
+                        functionsToInitialize.AddFirst(new JintFunctionDefinition(this, d));
                     }
                 }
             }
@@ -1153,7 +1152,7 @@ namespace Jint
 
             foreach (var f in functionsToInitialize)
             {
-                var fn = f.Id.Name;
+                var fn = f.Function.Id.Name;
                 var fo = realm.Intrinsics.Function.InstantiateFunctionObject(f, lexEnv);
                 if (varEnvRec is GlobalEnvironmentRecord ger)
                 {

Jint/EsprimaExtensions.cs

@@ -249,16 +249,13 @@ namespace Jint
             {
                 ExceptionHelper.ThrowSyntaxError(engine.Realm);
             }
-            var functionDefinition = new JintFunctionDefinition(engine, function);
-            var functionThisMode = functionDefinition.Strict || engine._isStrict
-                ? FunctionThisMode.Strict
-                : FunctionThisMode.Global;
 
+            var functionDefinition = new JintFunctionDefinition(engine, function);
             var closure = new ScriptFunctionInstance(
                 engine,
                 functionDefinition,
                 engine.ExecutionContext.LexicalEnvironment,
-                functionThisMode,
+                functionDefinition.ThisMode,
                 prototype);
 
             closure.MakeMethod(obj);

Jint/Native/Function/FunctionConstructor.cs

@@ -4,6 +4,7 @@ using Jint.Native.Object;
 using Jint.Runtime;
 using Jint.Runtime.Descriptors;
 using Jint.Runtime.Environments;
+using Jint.Runtime.Interpreter;
 
 namespace Jint.Native.Function
 {
@@ -124,13 +125,13 @@ namespace Jint.Native.Function
         /// <summary>
         /// https://tc39.es/ecma262/#sec-runtime-semantics-instantiatefunctionobject
         /// </summary>
-        internal FunctionInstance InstantiateFunctionObject(FunctionDeclaration functionDeclaration, EnvironmentRecord env)
+        internal FunctionInstance InstantiateFunctionObject(JintFunctionDefinition functionDeclaration, EnvironmentRecord env)
         {
             var functionObject = new ScriptFunctionInstance(
                 Engine,
                 functionDeclaration,
                 env,
-                functionDeclaration.Strict || _engine._isStrict)
+                functionDeclaration.ThisMode)
             {
                 _realm = _realm
             };

Jint/Native/Function/FunctionInstance.cs

@@ -337,6 +337,7 @@ namespace Jint.Native.Function
             localEnv.BindThisValue(thisValue);
         }
 
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal Completion OrdinaryCallEvaluateBody(
             JsValue[] arguments,
             ExecutionContext calleeContext)
@@ -347,11 +348,9 @@ namespace Jint.Native.Function
                 calleeContext.LexicalEnvironment);
 
             var result = _functionDefinition.Execute();
-            var value = result.GetValueOrDefault().Clone();
-
             argumentsInstance?.FunctionWasCalled();
 
-            return new Completion(result.Type, value, result.Identifier, result.Location);
+            return result;
         }
 
         /// <summary>

Jint/Runtime/Interpreter/Expressions/JintFunctionExpression.cs

@@ -1,4 +1,5 @@
 using Esprima.Ast;
+using Jint.Native;
 using Jint.Native.Function;
 using Jint.Runtime.Environments;
 
@@ -16,17 +17,18 @@ namespace Jint.Runtime.Interpreter.Expressions
 
         protected override object EvaluateInternal()
         {
-            var funcEnv = JintEnvironment.NewDeclarativeEnvironment(_engine, _engine.ExecutionContext.LexicalEnvironment);
+            return GetValue();
+        }
 
-            var functionThisMode = _function.Strict || _engine._isStrict
-                ? FunctionThisMode.Strict
-                : FunctionThisMode.Global;
+        public override JsValue GetValue()
+        {
+            var funcEnv = JintEnvironment.NewDeclarativeEnvironment(_engine, _engine.ExecutionContext.LexicalEnvironment);
 
             var closure = new ScriptFunctionInstance(
                 _engine,
                 _function,
                 funcEnv,
-                functionThisMode);
+                _function.ThisMode);
 
             closure.MakeConstructor();
 

Jint/Runtime/Interpreter/Expressions/JintObjectExpression.cs

@@ -26,6 +26,7 @@ namespace Jint.Runtime.Interpreter.Expressions
             internal readonly string? _key;
             private JsString? _keyJsString;
             internal readonly Property _value;
+            private JintFunctionDefinition? _functionDefinition;
 
             public ObjectProperty(string? key, Property property)
             {
@@ -34,6 +35,23 @@ namespace Jint.Runtime.Interpreter.Expressions
             }
 
             public JsString? KeyJsString => _keyJsString ??= _key != null ? JsString.Create(_key) : null;
+
+            public JintFunctionDefinition GetFunctionDefinition(Engine engine)
+            {
+                if (_functionDefinition is not null)
+                {
+                    return _functionDefinition;
+                }
+
+                var function = _value.Value as IFunction;
+                if (function is null)
+                {
+                    ExceptionHelper.ThrowSyntaxError(engine.Realm);
+                }
+
+                _functionDefinition = new JintFunctionDefinition(engine, function);
+                return _functionDefinition;
+            }
         }
 
         public JintObjectExpression(Engine engine, ObjectExpression expression) : base(engine, expression)
@@ -134,7 +152,6 @@ namespace Jint.Runtime.Interpreter.Expressions
         private object BuildObjectNormal()
         {
             var obj = _engine.Realm.Intrinsics.Object.Construct(_properties.Length);
-            bool isStrictModeCode = _engine._isStrict || StrictModeScope.IsStrictModeCode;
 
             for (var i = 0; i < _properties.Length; i++)
             {
@@ -177,17 +194,12 @@ namespace Jint.Runtime.Interpreter.Expressions
                 }
                 else if (property.Kind == PropertyKind.Get || property.Kind == PropertyKind.Set)
                 {
-                    var function = property.Value as IFunction;
-                    if (function is null)
-                    {
-                        ExceptionHelper.ThrowSyntaxError(_engine.Realm);
-                    }
-
+                    var function = objectProperty.GetFunctionDefinition(_engine);
                     var closure = new ScriptFunctionInstance(
                         _engine,
                         function,
                         _engine.ExecutionContext.LexicalEnvironment,
-                        isStrictModeCode);
+                        function.ThisMode);
 
                     closure.SetFunctionName(propName, property.Kind == PropertyKind.Get ? "get" : "set");
                     closure.MakeMethod(obj);

Jint/Runtime/Interpreter/Expressions/JintSequenceExpression.cs

@@ -15,20 +15,21 @@ namespace Jint.Runtime.Interpreter.Expressions
         protected override void Initialize()
         {
             var expression = (SequenceExpression) _expression;
-            _expressions = new JintExpression[expression.Expressions.Count];
-            for (var i = 0; i < expression.Expressions.Count; i++)
+            ref readonly var expressions = ref expression.Expressions;
+            var temp = new JintExpression[expressions.Count];
+            for (var i = 0; i < (uint) temp.Length; i++)
             {
-                _expressions[i] = Build(_engine, expression.Expressions[i]);
+                temp[i] = Build(_engine, expressions[i]);
             }
+
+            _expressions = temp;
         }
 
         protected override object EvaluateInternal()
         {
             var result = Undefined.Instance;
-            var expressions = _expressions;
-            for (var i = 0; i < (uint) expressions.Length; i++)
+            foreach (var expression in _expressions)
             {
-                var expression = expressions[i];
                 result = expression.GetValue();
             }
 

Jint/Runtime/Interpreter/JintFunctionDefinition.cs

@@ -31,23 +31,10 @@ namespace Jint.Runtime.Interpreter
             Function = function;
             Name = !string.IsNullOrEmpty(function.Id?.Name) ? function.Id.Name : null;
             Strict = function.Strict;
-
-            if (!Strict && !function.Expression)
-            {
-                // Esprima doesn't detect strict at the moment for
-                // language/expressions/object/method-definition/name-invoke-fn-strict.js
-                var blockStatement = (BlockStatement) function.Body;
-                ref readonly var statements = ref blockStatement.Body;
-                for (int i = 0; i < statements.Count; ++i)
-                {
-                    if (statements[i] is Directive d && d.Directiv == "use strict")
-                    {
-                        Strict = true;
-                    }
-                }
-            }
         }
 
+        public FunctionThisMode ThisMode => Strict || _engine._isStrict ? FunctionThisMode.Strict : FunctionThisMode.Global;
+
         internal Completion Execute()
         {
             if (Function.Expression)
@@ -77,7 +64,7 @@ namespace Jint.Runtime.Interpreter
             public bool HasParameterExpressions;
             public bool ArgumentsObjectNeeded;
             public List<Key> VarNames;
-            public LinkedList<FunctionDeclaration> FunctionsToInitialize;
+            public LinkedList<JintFunctionDefinition> FunctionsToInitialize;
             public readonly HashSet<Key> FunctionNames = new HashSet<Key>();
             public LexicalVariableDeclaration[] LexicalDeclarations = Array.Empty<LexicalVariableDeclaration>();
             public HashSet<Key> ParameterBindings;
@@ -107,18 +94,18 @@ namespace Jint.Runtime.Interpreter
             var lexicalNames = hoistingScope._lexicalNames;
             state.VarNames = hoistingScope._varNames;
 
-            LinkedList<FunctionDeclaration> functionsToInitialize = null;
+            LinkedList<JintFunctionDefinition> functionsToInitialize = null;
 
             if (functionDeclarations != null)
             {
-                functionsToInitialize = new LinkedList<FunctionDeclaration>();
+                functionsToInitialize = new LinkedList<JintFunctionDefinition>();
                 for (var i = functionDeclarations.Count - 1; i >= 0; i--)
                 {
                     var d = functionDeclarations[i];
                     var fn = d.Id.Name;
                     if (state.FunctionNames.Add(fn))
                     {
-                        functionsToInitialize.AddFirst(d);
+                        functionsToInitialize.AddFirst(new JintFunctionDefinition(_engine, d));
                     }
                 }
             }

Jint/Runtime/Interpreter/JintStatementList.cs

@@ -61,7 +61,7 @@ namespace Jint.Runtime.Interpreter
             JintStatement s = null;
             var c = new Completion(CompletionType.Normal, null, null, _engine._lastSyntaxNode?.Location ?? default);
             Completion sl = c;
-            
+
             // The value of a StatementList is the value of the last value-producing item in the StatementList
             JsValue lastValue = null;
             try
@@ -112,6 +112,7 @@ namespace Jint.Runtime.Interpreter
         /// https://tc39.es/ecma262/#sec-blockdeclarationinstantiation
         /// </summary>
         internal static void BlockDeclarationInstantiation(
+            Engine engine,
             EnvironmentRecord env,
             List<Declaration> declarations)
         {
@@ -138,7 +139,8 @@ namespace Jint.Runtime.Interpreter
                 if (d is FunctionDeclaration functionDeclaration)
                 {
                     var fn = functionDeclaration.Id!.Name;
-                    var fo = env._engine.Realm.Intrinsics.Function.InstantiateFunctionObject(functionDeclaration, env);
+                    var functionDefinition = new JintFunctionDefinition(engine, functionDeclaration);
+                    var fo = env._engine.Realm.Intrinsics.Function.InstantiateFunctionObject(functionDefinition, env);
                     envRec.InitializeBinding(fn, fo);
                 }
             }

Jint/Runtime/Interpreter/Statements/JintBlockStatement.cs

@@ -28,7 +28,7 @@ namespace Jint.Runtime.Interpreter.Statements
             {
                 oldEnv = _engine.ExecutionContext.LexicalEnvironment;
                 var blockEnv = JintEnvironment.NewDeclarativeEnvironment(_engine, _engine.ExecutionContext.LexicalEnvironment);
-                JintStatementList.BlockDeclarationInstantiation(blockEnv, _lexicalDeclarations);
+                JintStatementList.BlockDeclarationInstantiation(_engine, blockEnv, _lexicalDeclarations);
                 _engine.UpdateLexicalEnvironment(blockEnv);
             }
 
@@ -38,7 +38,7 @@ namespace Jint.Runtime.Interpreter.Statements
             {
                 _engine.UpdateLexicalEnvironment(oldEnv);
             }
-            
+
             return blockValue;
         }
     }

Jint/Runtime/Interpreter/Statements/JintSwitchBlock.cs

@@ -51,7 +51,7 @@ namespace Jint.Runtime.Interpreter.Statements
                 {
                     oldEnv = _engine.ExecutionContext.LexicalEnvironment;
                     var blockEnv = JintEnvironment.NewDeclarativeEnvironment(_engine, oldEnv);
-                    JintStatementList.BlockDeclarationInstantiation(blockEnv, clause.LexicalDeclarations);
+                    JintStatementList.BlockDeclarationInstantiation(_engine, blockEnv, clause.LexicalDeclarations);
                     _engine.UpdateLexicalEnvironment(blockEnv);
                 }
 
@@ -95,7 +95,7 @@ namespace Jint.Runtime.Interpreter.Statements
                 {
                     oldEnv = _engine.ExecutionContext.LexicalEnvironment;
                     var blockEnv = JintEnvironment.NewDeclarativeEnvironment(_engine, oldEnv);
-                    JintStatementList.BlockDeclarationInstantiation(blockEnv, defaultCase.LexicalDeclarations);
+                    JintStatementList.BlockDeclarationInstantiation(_engine, blockEnv, defaultCase.LexicalDeclarations);
                     _engine.UpdateLexicalEnvironment(blockEnv);
                 }