Get rid of GETINDEXSUPER and related

Those invokedynamic bridges were never being executed because they
where generated only in the combination of `super` with an integer
index in optimized functions. However, to make `super` work, the
function is marked with "requires activation", which disables the
optimizer, which means that the combination
`SUPER_PROPERTY_ACCESS` and `ISNUMBER_PROP` actually never occurs.

We removed all the dead code to improve coverage.

Co-authored-by: Satish Srinivasan <satish.srinivasan@servicenow.com>

rhino/src/main/java/org/mozilla/javascript/optimizer/BodyCodegen.java

@@ -1773,11 +1773,7 @@ private void finishGetElemGeneration(Node node, Node child) {
 
         if (node.getIntProp(Node.SUPER_PROPERTY_ACCESS, 0) == 1) {
             cfw.addALoad(thisObjLocal);
-            if (node.getIntProp(Node.ISNUMBER_PROP, -1) != -1) {
-                addDynamicInvoke("PROP:GETINDEXSUPER", Signatures.PROP_GET_INDEX_SUPER);
-            } else {
-                addDynamicInvoke("PROP:GETELEMENTSUPER", Signatures.PROP_GET_ELEMENT_SUPER);
-            }
+            addDynamicInvoke("PROP:GETELEMENTSUPER", Signatures.PROP_GET_ELEMENT_SUPER);
         } else {
             if (node.getIntProp(Node.ISNUMBER_PROP, -1) != -1) {
                 addDynamicInvoke("PROP:GETINDEX", Signatures.PROP_GET_INDEX);
@@ -4440,48 +4436,44 @@ private void visitSetElem(int type, Node node, Node child) {
         boolean indexIsNumber = (node.getIntProp(Node.ISNUMBER_PROP, -1) != -1);
         boolean isSuper = node.getIntProp(Node.SUPER_PROPERTY_ACCESS, 0) == 1;
         if (type == Token.SETELEM_OP) {
-            if (indexIsNumber) {
+            if (isSuper) {
+                // indexIsNumber will never be true because functions with super always require
+                // activation, and when they do we not optimize them
+
+                // stack: ... object object indexObject
+                //        -> ... object indexObject object indexObject
+                cfw.add(ByteCode.DUP_X1);
+                cfw.addALoad(contextLocal);
+                cfw.addALoad(variableObjectLocal);
+                cfw.addALoad(thisObjLocal);
+                addDynamicInvoke("PROP:GETELEMENTSUPER", Signatures.PROP_GET_ELEMENT_SUPER);
+            } else if (indexIsNumber) {
                 // stack: ... object object number
                 //        -> ... object number object number
                 cfw.add(ByteCode.DUP2_X1);
                 cfw.addALoad(contextLocal);
                 cfw.addALoad(variableObjectLocal);
-                if (isSuper) {
-                    cfw.addALoad(thisObjLocal);
-                    addDynamicInvoke("PROP:GETINDEXSUPER", Signatures.PROP_GET_INDEX_SUPER);
-                } else {
-                    addDynamicInvoke("PROP:GETINDEX", Signatures.PROP_GET_INDEX);
-                }
+                addDynamicInvoke("PROP:GETINDEX", Signatures.PROP_GET_INDEX);
             } else {
                 // stack: ... object object indexObject
                 //        -> ... object indexObject object indexObject
                 cfw.add(ByteCode.DUP_X1);
                 cfw.addALoad(contextLocal);
                 cfw.addALoad(variableObjectLocal);
-                if (isSuper) {
-                    cfw.addALoad(thisObjLocal);
-                    addDynamicInvoke("PROP:GETELEMENTSUPER", Signatures.PROP_GET_ELEMENT_SUPER);
-                } else {
-                    addDynamicInvoke("PROP:GETELEMENT", Signatures.PROP_GET_ELEMENT);
-                }
+                addDynamicInvoke("PROP:GETELEMENT", Signatures.PROP_GET_ELEMENT);
             }
         }
         generateExpression(child, node);
         cfw.addALoad(contextLocal);
         cfw.addALoad(variableObjectLocal);
         if (isSuper) {
+            // Again, we will never have super && indexIsNumber together
             cfw.addALoad(thisObjLocal);
-            if (indexIsNumber) {
-                addDynamicInvoke("PROP:SETINDEXSUPER", Signatures.PROP_SET_INDEX_SUPER);
-            } else {
-                addDynamicInvoke("PROP:SETELEMENTSUPER", Signatures.PROP_SET_ELEMENT_SUPER);
-            }
+            addDynamicInvoke("PROP:SETELEMENTSUPER", Signatures.PROP_SET_ELEMENT_SUPER);
+        } else if (indexIsNumber) {
+            addDynamicInvoke("PROP:SETINDEX", Signatures.PROP_SET_INDEX);
         } else {
-            if (indexIsNumber) {
-                addDynamicInvoke("PROP:SETINDEX", Signatures.PROP_SET_INDEX);
-            } else {
-                addDynamicInvoke("PROP:SETELEMENT", Signatures.PROP_SET_ELEMENT);
-            }
+            addDynamicInvoke("PROP:SETELEMENT", Signatures.PROP_SET_ELEMENT);
         }
     }
 

rhino/src/main/java/org/mozilla/javascript/optimizer/Bootstrapper.java

@@ -133,9 +133,6 @@ private static Operation parseOperation(String name) throws NoSuchMethodExceptio
                 case "GETINDEX":
                     // Same but the value is definitely a numeric index
                     return RhinoOperation.GETINDEX.withNamespace(StandardNamespace.PROPERTY);
-                case "GETINDEXSUPER":
-                    // Same but the value is definitely a numeric index
-                    return RhinoOperation.GETINDEXSUPER.withNamespace(StandardNamespace.PROPERTY);
                 case "SET":
                     // Set an object property with a constant name
                     return StandardOperation.SET
@@ -155,9 +152,6 @@ private static Operation parseOperation(String name) throws NoSuchMethodExceptio
                 case "SETINDEX":
                     // Same but the property name is definitely a number
                     return RhinoOperation.SETINDEX.withNamespace(StandardNamespace.PROPERTY);
-                case "SETINDEXSUPER":
-                    // Same but the property name is definitely a number
-                    return RhinoOperation.SETINDEXSUPER.withNamespace(StandardNamespace.PROPERTY);
             }
         } else if ("NAME".equals(namespaceName)) {
             switch (opName) {

rhino/src/main/java/org/mozilla/javascript/optimizer/DefaultLinker.java

@@ -125,16 +125,12 @@ private GuardedInvocation getPropertyInvocation(
             mh = lookup.findStatic(ScriptRuntime.class, "getSuperElem", mType);
         } else if (op.isOperation(RhinoOperation.GETINDEX)) {
             mh = lookup.findStatic(ScriptRuntime.class, "getObjectIndex", mType);
-        } else if (op.isOperation(RhinoOperation.GETINDEXSUPER)) {
-            mh = lookup.findStatic(ScriptRuntime.class, "getSuperIndex", mType);
         } else if (op.isOperation(RhinoOperation.SETELEMENT)) {
             mh = lookup.findStatic(ScriptRuntime.class, "setObjectElem", mType);
         } else if (op.isOperation(RhinoOperation.SETELEMENTSUPER)) {
             mh = lookup.findStatic(ScriptRuntime.class, "setSuperElem", mType);
         } else if (op.isOperation(RhinoOperation.SETINDEX)) {
             mh = lookup.findStatic(ScriptRuntime.class, "setObjectIndex", mType);
-        } else if (op.isOperation(RhinoOperation.SETINDEXSUPER)) {
-            mh = lookup.findStatic(ScriptRuntime.class, "setSuperIndex", mType);
         }
 
         if (mh != null) {

rhino/src/main/java/org/mozilla/javascript/optimizer/RhinoOperation.java

@@ -16,14 +16,12 @@ public enum RhinoOperation implements Operation {
     GETELEMENT,
     GETELEMENTSUPER,
     GETINDEX,
-    GETINDEXSUPER,
     SETSTRICT,
     SETCONST,
     SETSUPER,
     SETELEMENT,
     SETELEMENTSUPER,
     SETINDEX,
-    SETINDEXSUPER,
     ADD,
     EQ,
     SHALLOWEQ,

rhino/src/main/java/org/mozilla/javascript/optimizer/Signatures.java

@@ -61,18 +61,6 @@ interface Signatures {
                     + "Lorg/mozilla/javascript/Scriptable;"
                     + ")Ljava/lang/Object;";
 
-    /**
-     * PROP:GETINDEXSUPER: Get a property from super based on a numeric index. Falls back to
-     * ScriptRuntime.getSuperIndex.
-     */
-    String PROP_GET_INDEX_SUPER =
-            "(Ljava/lang/Object;"
-                    + "D"
-                    + "Lorg/mozilla/javascript/Context;"
-                    + "Lorg/mozilla/javascript/Scriptable;"
-                    + "Ljava/lang/Object;"
-                    + ")Ljava/lang/Object;";
-
     /**
      * PROP:GETELEMENT: Get a property from an object based on an element ID, which could be a
      * string, number, or symbol. Falls back to ScriptRuntime.getObjectElem.
@@ -129,19 +117,6 @@ interface Signatures {
                     + "Lorg/mozilla/javascript/Scriptable;"
                     + ")Ljava/lang/Object;";
 
-    /**
-     * PROP:SETINDEXSUPER: Set a property on super based on a numeric index. Falls back to
-     * ScriptRuntime.setSuperIndex.
-     */
-    String PROP_SET_INDEX_SUPER =
-            "(Ljava/lang/Object;"
-                    + "D"
-                    + "Ljava/lang/Object;"
-                    + "Lorg/mozilla/javascript/Context;"
-                    + "Lorg/mozilla/javascript/Scriptable;"
-                    + "Ljava/lang/Object;"
-                    + ")Ljava/lang/Object;";
-
     /**
      * PROP:SETELEMENT: Set a property on an object based on an identifier. Falls back to
      * ScriptRuntime.setObjectElem.

rhino/src/test/java/org/mozilla/javascript/SuperTest.java

@@ -768,6 +768,23 @@ void modifyOperatorByName() {
             Utils.assertWithAllOptimizationLevelsES6("proto1:proto", script);
         }
 
+        @Test
+        void modifyOperatorByKey() {
+            // Equivalent to `super[x] = super[x] + 1`, so reads from super, writes in this
+            String script =
+                    ""
+                            + "const xAsStr = 'x';\n"
+                            + "var proto = { x: 'proto' };"
+                            + "var object = {\n"
+                            + "  x: 'obj',\n"
+                            + "  f() { super[xAsStr] += '1'; }\n"
+                            + "};\n"
+                            + "Object.setPrototypeOf(object, proto);\n"
+                            + "object.f();"
+                            + "object.x + ':' + proto.x";
+            Utils.assertWithAllOptimizationLevelsES6("proto1:proto", script);
+        }
+
         @Test
         void deleteNotAllowed() {
             String script =