Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace JsValue[] arguments with Arguments struct #1135

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions Jint.Benchmark/UncacheableExpressionsBenchmark.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Text;
using BenchmarkDotNet.Attributes;
using Jint.Native;
using Jint.Runtime;
using Newtonsoft.Json;
using Undefined = Jint.Native.Undefined;

Expand All @@ -19,7 +20,7 @@ public class UncacheableExpressionsBenchmark
private Document doc;

private string targetObject;
private JsValue[] targetJsObject;
private Arguments targetJsObject;

private const string NonArrowFunctionScript = @"
function output(d) {
Expand Down Expand Up @@ -112,7 +113,7 @@ private void CreateEngine(string script)
engine = new Engine(InitializeEngine);
engine.Execute(script);
engine.Execute(targetObject);
targetJsObject = new[] {engine.GetValue("d")};
targetJsObject = new Arguments(engine.GetValue("d"));
}
}
}
}
8 changes: 4 additions & 4 deletions Jint.Tests.Test262/Test262Test.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ private Engine BuildTestExecutor(Test262File file)
engine.Execute(State.Sources["sta.js"]);

engine.SetValue("print",
new ClrFunctionInstance(engine, "print", (thisObj, args) => TypeConverter.ToString(args.At(0))));
new ClrFunctionInstance(engine, "print", (JsValue _, in Arguments args) => TypeConverter.ToString(args.At(0))));

var o = engine.Realm.Intrinsics.Object.Construct(Arguments.Empty);
o.FastSetProperty("evalScript", new PropertyDescriptor(new ClrFunctionInstance(engine, "evalScript",
(thisObj, args) =>
(JsValue _, in Arguments args) =>
{
if (args.Length > 1)
{
Expand All @@ -43,15 +43,15 @@ private Engine BuildTestExecutor(Test262File file)
}), true, true, true));

o.FastSetProperty("createRealm", new PropertyDescriptor(new ClrFunctionInstance(engine, "createRealm",
(thisObj, args) =>
(JsValue _, in Arguments _) =>
{
var realm = engine._host.CreateRealm();
realm.GlobalObject.Set("global", realm.GlobalObject);
return realm.GlobalObject;
}), true, true, true));

o.FastSetProperty("detachArrayBuffer", new PropertyDescriptor(new ClrFunctionInstance(engine, "detachArrayBuffer",
(thisObj, args) =>
(JsValue _, in Arguments args) =>
{
var buffer = (ArrayBufferInstance) args.At(0);
buffer.DetachArrayBuffer();
Expand Down
6 changes: 3 additions & 3 deletions Jint.Tests/Runtime/Domain/UuidConstructor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ private UuidConstructor(Engine engine) : base(engine, engine.Realm, _functionNam
{
}

private JsValue Parse(JsValue @this, JsValue[] arguments)
private JsValue Parse(JsValue thisObject, in Arguments arguments)
{
switch (arguments.At(0))
{
Expand Down Expand Up @@ -55,7 +55,7 @@ public static UuidConstructor CreateUuidConstructor(Engine engine)
return obj;
}

public override JsValue Call(JsValue thisObject, JsValue[] arguments) => Construct(arguments, null);
public override JsValue Call(JsValue thisObject, in Arguments arguments) => Construct(arguments, null);

public void Configure()
{
Expand All @@ -65,6 +65,6 @@ public void Configure()

public UuidInstance Construct(JsUuid uuid) => new UuidInstance(Engine) { PrimitiveValue = uuid, _prototype = PrototypeObject };

public ObjectInstance Construct(JsValue[] arguments, JsValue newTarget) => Construct(Guid.NewGuid());
public ObjectInstance Construct(in Arguments arguments, JsValue newTarget) => Construct(Guid.NewGuid());
}
}
4 changes: 2 additions & 2 deletions Jint.Tests/Runtime/Domain/UuidPrototype.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ private UuidInstance EnsureUuidInstance(JsValue thisObj)
});
}

private JsValue ToGuidString(JsValue thisObj, JsValue[] arguments) => EnsureUuidInstance(thisObj).PrimitiveValue.ToString();
private JsValue ToGuidString(JsValue thisObj, in Arguments arguments) => EnsureUuidInstance(thisObj).PrimitiveValue.ToString();

private JsValue ValueOf(JsValue thisObj, JsValue[] arguments) => EnsureUuidInstance(thisObj).PrimitiveValue;
private JsValue ValueOf(JsValue thisObj, in Arguments arguments) => EnsureUuidInstance(thisObj).PrimitiveValue;

public static UuidPrototype CreatePrototypeObject(Engine engine, UuidConstructor ctor)
{
Expand Down
4 changes: 2 additions & 2 deletions Jint.Tests/Runtime/FunctionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ function test() {
";
var engine = new Engine();
engine
.SetValue("testFunction", new ClrFunctionInstance(engine, "testFunction", (thisValue, args) =>
.SetValue("testFunction", new ClrFunctionInstance(engine, "testFunction", (JsValue thisValue, in Arguments args) =>
{
return engine.Invoke(thisValue, "then", new[] { Undefined.Instance, args.At(0) });
return engine.Invoke(thisValue, "then", new Arguments(Undefined.Instance, args.At(0)));
}))
.SetValue("assertEqual", new Action<object, object>((a, b) => Assert.Equal(b, a)))
.Execute(script);
Expand Down
2 changes: 1 addition & 1 deletion Jint.Tests/Runtime/ModuleTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ public void ShouldAllowInvokeUserDefinedClass()
_engine.AddModule("user", "export class UserDefined { constructor(v) { this._v = v; } hello(c) { return `hello ${this._v}${c}`; } }");
var ctor = _engine.ImportModule("user").Get("UserDefined");
var instance = _engine.Construct(ctor, JsString.Create("world"));
var result = instance.GetMethod("hello").Call(instance, JsString.Create("!"));
var result = instance.GetMethod("hello").Call(instance, new Arguments("!"));

Assert.Equal("hello world!", result);
}
Expand Down
6 changes: 3 additions & 3 deletions Jint.Tests/Runtime/NullPropagation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ public bool TryGetCallable(Engine engine, object callee, out JsValue value)
var name = reference.GetReferencedName().AsString();
if (name == "filter")
{
value = new ClrFunctionInstance(engine, "map", (thisObj, values) => engine.Realm.Intrinsics.Array.ArrayCreate(0));
value = new ClrFunctionInstance(engine, "map", (JsValue _, in Arguments _) => engine.Realm.Intrinsics.Array.ArrayCreate(0));
return true;
}
}

value = new ClrFunctionInstance(engine, "anonymous", (thisObj, values) => thisObj);
value = new ClrFunctionInstance(engine, "anonymous", (JsValue thisObj, in Arguments _) => thisObj);
return true;
}

Expand Down Expand Up @@ -147,4 +147,4 @@ public void NullPropagationShouldNotAffectOperators()
Assert.True(jsObject.Get("has_emptyfield_not_null").AsBoolean());
}
}
}
}
8 changes: 4 additions & 4 deletions Jint/Collections/DictionarySlim.cs
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,9 @@ public bool MoveNext()
return true;
}

public KeyValuePair<TKey, TValue> Current => _current;
public readonly KeyValuePair<TKey, TValue> Current => _current;

object IEnumerator.Current => _current;
readonly object IEnumerator.Current => _current;

void IEnumerator.Reset()
{
Expand All @@ -279,7 +279,7 @@ void IEnumerator.Reset()
_current = default;
}

public void Dispose() { }
public readonly void Dispose() { }
}

internal static class HashHelpers
Expand All @@ -295,4 +295,4 @@ internal static int PowerOf2(int v)
}
}
}
}
}
8 changes: 4 additions & 4 deletions Jint/Collections/StringDictionarySlim.cs
Original file line number Diff line number Diff line change
Expand Up @@ -274,9 +274,9 @@ public bool MoveNext()
return true;
}

public KeyValuePair<Key, TValue> Current => _current;
public readonly KeyValuePair<Key, TValue> Current => _current;

object IEnumerator.Current => _current;
readonly object IEnumerator.Current => _current;

void IEnumerator.Reset()
{
Expand All @@ -285,7 +285,7 @@ void IEnumerator.Reset()
_current = default;
}

public void Dispose() { }
public readonly void Dispose() { }
}

internal static class HashHelpers
Expand Down Expand Up @@ -314,4 +314,4 @@ public DictionarySlimDebugView(StringDictionarySlim<V> dictionary)
public KeyValuePair<Key, V>[] Items => _dictionary.ToArray();
}
}
}
}
2 changes: 2 additions & 0 deletions Jint/Constraints/TimeConstraint2.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ namespace Jint.Constraints
internal sealed class TimeConstraint2 : IConstraint
{
private readonly TimeSpan _timeout;
#pragma warning disable IDISP006
private CancellationTokenSource cts;
#pragma warning restore IDISP006

public TimeConstraint2(TimeSpan timeout)
{
Expand Down
28 changes: 13 additions & 15 deletions Jint/Engine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ public ManualPromise RegisterPromise()

Action<JsValue> SettleWith(FunctionInstance settle) => value =>
{
settle.Call(JsValue.Undefined, new[] {value});
settle.Call(JsValue.Undefined, new Arguments(value));
RunAvailableContinuations();
};

Expand Down Expand Up @@ -602,14 +602,13 @@ public JsValue Invoke(JsValue value, object thisObj, object[] arguments)

JsValue DoInvoke()
{
var items = _jsValueArrayPool.RentArray(arguments.Length);
using var items = _jsValueArrayPool.RentArray(arguments.Length);
for (var i = 0; i < arguments.Length; ++i)
{
items[i] = JsValue.FromObject(this, arguments[i]);
}

var result = callable.Call(JsValue.FromObject(this, thisObj), items);
_jsValueArrayPool.ReturnArray(items);
var result = callable.Call(JsValue.FromObject(this, thisObj), new Arguments(items._array, arguments.Length));
return result;
}

Expand Down Expand Up @@ -646,7 +645,7 @@ private T ExecuteWithConstraints<T>(bool strict, Func<T> callback)
/// <summary>
/// https://tc39.es/ecma262/#sec-invoke
/// </summary>
internal JsValue Invoke(JsValue v, JsValue p, JsValue[] arguments)
internal JsValue Invoke(JsValue v, JsValue p, Arguments arguments)
{
var ownsContext = _activeEvaluationContext is null;
_activeEvaluationContext ??= new EvaluationContext(this);
Expand Down Expand Up @@ -876,7 +875,7 @@ private void GlobalDeclarationInstantiation(
/// </summary>
internal ArgumentsInstance FunctionDeclarationInstantiation(
FunctionInstance functionInstance,
JsValue[] argumentsList)
Arguments argumentsList)
{
var calleeContext = ExecutionContext;
var func = functionInstance._functionDefinition;
Expand All @@ -891,8 +890,7 @@ internal ArgumentsInstance FunctionDeclarationInstantiation(
var hasParameterExpressions = configuration.HasParameterExpressions;

var canInitializeParametersOnDeclaration = simpleParameterList && !configuration.HasDuplicates;
env.InitializeParameters(parameterNames, hasDuplicates,
canInitializeParametersOnDeclaration ? argumentsList : null);
env.InitializeParameters(parameterNames, hasDuplicates, canInitializeParametersOnDeclaration ? argumentsList : null);

ArgumentsInstance ao = null;
if (configuration.ArgumentsObjectNeeded)
Expand Down Expand Up @@ -1015,14 +1013,14 @@ internal ArgumentsInstance FunctionDeclarationInstantiation(
private ArgumentsInstance CreateMappedArgumentsObject(
FunctionInstance func,
Key[] formals,
JsValue[] argumentsList,
Arguments argumentsList,
DeclarativeEnvironmentRecord envRec,
bool hasRestParameter)
{
return _argumentsInstancePool.Rent(func, formals, argumentsList, envRec, hasRestParameter);
}

private ArgumentsInstance CreateUnmappedArgumentsObject(JsValue[] argumentsList)
private ArgumentsInstance CreateUnmappedArgumentsObject(in Arguments argumentsList)
{
return _argumentsInstancePool.Rent(argumentsList);
}
Expand Down Expand Up @@ -1211,7 +1209,7 @@ internal void UpdateVariableEnvironment(EnvironmentRecord newEnv)
_executionContexts.ReplaceTopVariableEnvironment(newEnv);
}

internal JsValue Call(ICallable callable, JsValue thisObject, JsValue[] arguments, JintExpression expression)
internal JsValue Call(ICallable callable, JsValue thisObject, in Arguments arguments, JintExpression expression)
{
if (callable is FunctionInstance functionInstance)
{
Expand Down Expand Up @@ -1248,15 +1246,15 @@ ObjectInstance Callback()
ExceptionHelper.ThrowArgumentException(constructor + " is not a constructor");
}

return Construct(constructor, arguments, constructor, null);
return Construct(constructor, new Arguments(arguments, arguments.Length), constructor, null);
}

return ExecuteWithConstraints(Options.Strict, Callback);
}

internal ObjectInstance Construct(
JsValue constructor,
JsValue[] arguments,
Arguments arguments,
JsValue newTarget,
JintExpression expression)
{
Expand All @@ -1271,7 +1269,7 @@ internal ObjectInstance Construct(
internal JsValue Call(
FunctionInstance functionInstance,
JsValue thisObject,
JsValue[] arguments,
Arguments arguments,
JintExpression expression)
{
var callStackElement = new CallStackElement(functionInstance, expression, ExecutionContext);
Expand All @@ -1293,7 +1291,7 @@ internal JsValue Call(

private ObjectInstance Construct(
FunctionInstance functionInstance,
JsValue[] arguments,
Arguments arguments,
JsValue newTarget,
JintExpression expression)
{
Expand Down
2 changes: 2 additions & 0 deletions Jint/Jint.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
<IsPackable>true</IsPackable>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="IDisposableAnalyzers" Version="4.0.2" PrivateAssets="all" />
<PackageReference Include="ErrorProne.NET.Structs" Version="0.4.0-beta.1" PrivateAssets="all" />
<PackageReference Include="Esprima" Version="2.1.2" />
<PackageReference Include="IsExternalInit" Version="1.0.2" PrivateAssets="all" />
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1" PrivateAssets="all" />
Expand Down
2 changes: 1 addition & 1 deletion Jint/JsValueExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ public static JsValue Call(this JsValue value, params JsValue[] arguments)
{
ExceptionHelper.ThrowArgumentException(value + " is not callable");
}
return ((ICallable) value).Call(JsValue.Undefined, arguments);
return ((ICallable) value).Call(JsValue.Undefined, new Arguments(arguments, arguments.Length));
}

/// <summary>
Expand Down
12 changes: 6 additions & 6 deletions Jint/ModuleBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,21 +68,21 @@ public ModuleBuilder ExportType(string name, Type type)
return this;
}

public ModuleBuilder ExportFunction(string name, Func<JsValue[], JsValue> fn)
public ModuleBuilder ExportFunction(string name, Func<Arguments, JsValue> fn)
{
_exports.Add(name, new ClrFunctionInstance(_engine, name, (_, args) => fn(args)));
_exports.Add(name, new ClrFunctionInstance(_engine, name, (JsValue _, in Arguments args) => fn(args)));
return this;
}

public ModuleBuilder ExportFunction(string name, Func<JsValue> fn)
{
_exports.Add(name, new ClrFunctionInstance(_engine, name, (_, _) => fn()));
_exports.Add(name, new ClrFunctionInstance(_engine, name, (JsValue _, in Arguments _) => fn()));
return this;
}

public ModuleBuilder ExportFunction(string name, Action<JsValue[]> fn)
public ModuleBuilder ExportFunction(string name, Action<Arguments> fn)
{
_exports.Add(name, new ClrFunctionInstance(_engine, name, (_, args) =>
_exports.Add(name, new ClrFunctionInstance(_engine, name, (JsValue _, in Arguments args) =>
{
fn(args);
return JsValue.Undefined;
Expand All @@ -92,7 +92,7 @@ public ModuleBuilder ExportFunction(string name, Action<JsValue[]> fn)

public ModuleBuilder ExportFunction(string name, Action fn)
{
_exports.Add(name, new ClrFunctionInstance(_engine, name, (_, _) =>
_exports.Add(name, new ClrFunctionInstance(_engine, name, (JsValue _, in Arguments _) =>
{
fn();
return JsValue.Undefined;
Expand Down
Loading