Browse Source

Fix string-indexing optimizations against custom strings (#1870)

Marko Lahma 1 year ago
parent
commit
c42a63dea6
4 changed files with 23 additions and 34 deletions
  1. 20 9
      Jint.Tests.PublicInterface/RavenApiUsageTests.cs
  2. 3 6
      Jint/Engine.cs
  3. 0 11
      NuGet.config
  4. 0 8
      NuGet.release.config

+ 20 - 9
Jint.Tests.PublicInterface/RavenApiUsageTests.cs

@@ -69,12 +69,11 @@ public class RavenApiUsageTests
         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");
@@ -86,7 +85,7 @@ public class RavenApiUsageTests
         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"));
 
@@ -100,7 +99,7 @@ public class RavenApiUsageTests
         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())
@@ -144,6 +143,8 @@ public class RavenApiUsageTests
         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);
@@ -157,7 +158,7 @@ public class RavenApiUsageTests
         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"));
@@ -174,6 +175,9 @@ public class RavenApiUsageTests
         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]
@@ -203,16 +207,23 @@ public class RavenApiUsageTests
 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];

+ 3 - 6
Jint/Engine.cs

@@ -660,20 +660,17 @@ namespace Jint
             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;

+ 0 - 11
NuGet.config

@@ -1,11 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<configuration>
-  <packageSources>
-    <clear />
-    <add key="NuGet" value="https://api.nuget.org/v3/index.json" />
-    <!--
-    <add key="Esprima" value="https://www.myget.org/F/esprimadotnet/api/v3/index.json" />
-    -->
-  </packageSources>
-  <disabledPackageSources />
-</configuration>

+ 0 - 8
NuGet.release.config

@@ -1,8 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<configuration>
-  <packageSources>
-    <clear />
-    <add key="NuGet" value="https://api.nuget.org/v3/index.json" />
-  </packageSources>
-  <disabledPackageSources />
-</configuration>