Skip to content

Commit

Permalink
Fix string-indexing optimizations against custom strings (#1870)
Browse files Browse the repository at this point in the history
  • Loading branch information
lahma authored May 25, 2024
1 parent a7dd50d commit c42a63d
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 34 deletions.
29 changes: 20 additions & 9 deletions Jint.Tests.PublicInterface/RavenApiUsageTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,11 @@ public void CanInjectConstructedObjects()
var obj = new JsObject(engine);
obj.FastSetDataProperty("name", "test");

var array1 = new JsArray(engine, new JsValue[]
{
var array1 = new JsArray(engine, [
JsNumber.Create(1),
JsNumber.Create(2),
JsNumber.Create(3)
});
]);
engine.SetValue("array1", array1);

TestArrayAccess(engine, array1, "array1");
Expand All @@ -86,7 +85,7 @@ public void CanInjectConstructedObjects()
Assert.Equal(0, engine.Evaluate("emptyArray.length"));
Assert.Equal(1, engine.Evaluate("emptyArray.push(1); return emptyArray.length"));

engine.SetValue("emptyArray", new JsArray(engine, Array.Empty<JsValue>()));
engine.SetValue("emptyArray", new JsArray(engine, []));
Assert.Equal(0, engine.Evaluate("emptyArray.length"));
Assert.Equal(1, engine.Evaluate("emptyArray.push(1); return emptyArray.length"));

Expand All @@ -100,7 +99,7 @@ private static void TestArrayAccess(Engine engine, JsArray array, string name)
Assert.Equal(2, array.GetOwnProperty("1").Value);

array.Push(4);
array.Push(new JsValue[] { 5, 6 });
array.Push([5, 6]);

var i = 0;
foreach (var entry in array.GetEntries())
Expand Down Expand Up @@ -144,6 +143,8 @@ public void CanInheritCustomString()
var empty = new CustomString("");
engine.SetValue("empty", empty);

engine.SetValue("x", new CustomString("x", allowMaterialize: true));

var obj = new JsObject(engine);
obj.Set("name", new CustomString("the name"));
engine.SetValue("obj", obj);
Expand All @@ -157,7 +158,7 @@ public void CanInheritCustomString()
Assert.True(engine.Evaluate("array.includes('2')").AsBoolean());
Assert.True(engine.Evaluate("array.filter(x => x === '2').length > 0").AsBoolean());

engine.SetValue("objArray", new JsArray(engine, new JsValue[] { obj, obj }));
engine.SetValue("objArray", new JsArray(engine, [obj, obj]));
Assert.True(engine.Evaluate("objArray.filter(x => x.name === 'the name').length === 2").AsBoolean());

Assert.Equal(9, engine.Evaluate("str.length"));
Expand All @@ -174,6 +175,9 @@ public void CanInheritCustomString()
Assert.True(engine.Evaluate("empty.trim() === ''").AsBoolean());
Assert.True(engine.Evaluate("empty.trimStart() === ''").AsBoolean());
Assert.True(engine.Evaluate("empty.trimEnd() === ''").AsBoolean());

Assert.True(engine.Evaluate("str[1] === 'h'").AsBoolean());
Assert.True(engine.Evaluate("str[x] === undefined").AsBoolean());
}

[Fact]
Expand Down Expand Up @@ -203,16 +207,23 @@ public void CanResetCallStack()
file sealed class CustomString : JsString
{
private readonly string _value;
private readonly bool _allowMaterialize;

public CustomString(string value) : base(null)
public CustomString(string value, bool allowMaterialize = false) : base(null)
{
_value = value;
_allowMaterialize = allowMaterialize;
}

public override string ToString()
{
// when called we know that we couldn't use fast paths
throw new InvalidOperationException("I don't want to be materialized!");
if (!_allowMaterialize)
{
// when called we know that we couldn't use fast paths
throw new InvalidOperationException("I don't want to be materialized!");
}

return _value;
}

public override char this[int index] => _value[index];
Expand Down
9 changes: 3 additions & 6 deletions Jint/Engine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -660,20 +660,17 @@ private bool TryHandleStringValue(JsValue property, JsString s, ref ObjectInstan
if (property is JsNumber number && number.IsInteger())
{
var index = number.AsInteger();
var str = s._value;
if (index < 0 || index >= str.Length)
if (index < 0 || index >= s.Length)
{
jsValue = JsValue.Undefined;
return true;
}

jsValue = JsString.Create(str[index]);
jsValue = JsString.Create(s[index]);
return true;
}

if (property is JsString propertyString
&& propertyString._value.Length > 0
&& char.IsLower(propertyString._value[0]))
if (property is JsString { Length: > 0 } propertyString && char.IsLower(propertyString[0]))
{
// trying to find property that's always in prototype
o = Realm.Intrinsics.String.PrototypeObject;
Expand Down
11 changes: 0 additions & 11 deletions NuGet.config

This file was deleted.

8 changes: 0 additions & 8 deletions NuGet.release.config

This file was deleted.

0 comments on commit c42a63d

Please sign in to comment.