From 3fdcff8ad3204ce27f8ba33170cab12350bcdcbb Mon Sep 17 00:00:00 2001 From: "nboyd%atg.com" Date: Sun, 18 Oct 2009 02:34:50 +0000 Subject: [PATCH] Fix Bug 508964 - Update Arguments object to match ES5 spec Patch from Raphael Speyer git-svn-id: svn://10.0.0.236/trunk@258712 18797224-902f-48f8-a5cc-f745e15eee43 --- .../src/org/mozilla/javascript/Arguments.java | 245 ++++++++++++++---- .../mozilla/javascript/ScriptableObject.java | 18 +- .../rhino/testsrc/doctests/arguments.doctest | 120 +++++++++ mozilla/js/rhino/testsrc/opt-1.tests | 3 - mozilla/js/rhino/testsrc/opt0.tests | 3 - mozilla/js/rhino/testsrc/opt9.tests | 3 - 6 files changed, 322 insertions(+), 70 deletions(-) create mode 100644 mozilla/js/rhino/testsrc/doctests/arguments.doctest diff --git a/mozilla/js/rhino/src/org/mozilla/javascript/Arguments.java b/mozilla/js/rhino/src/org/mozilla/javascript/Arguments.java index a6f7eea0b54..def717c7958 100644 --- a/mozilla/js/rhino/src/org/mozilla/javascript/Arguments.java +++ b/mozilla/js/rhino/src/org/mozilla/javascript/Arguments.java @@ -51,13 +51,15 @@ final class Arguments extends IdScriptableObject { static final long serialVersionUID = 4275508002492040609L; + private static final String FTAG = "Arguments"; + public Arguments(NativeCall activation) { this.activation = activation; Scriptable parent = activation.getParentScope(); setParentScope(parent); - setPrototype(ScriptableObject.getObjectPrototype(parent)); + setPrototype(ScriptableObject.getArrayPrototype(parent)); args = activation.originalArgs; lengthObj = Integer.valueOf(args.length); @@ -65,6 +67,15 @@ final class Arguments extends IdScriptableObject NativeFunction f = activation.function; calleeObj = f; + Scriptable topLevel = getTopLevelScope(parent); + objectCtor = (BaseFunction) getProperty(topLevel, "Object"); + + constructor = objectCtor; + toString = new IdFunctionObject(this, FTAG, Id_toString, "toString", + 0, parent); + toLocaleString = new IdFunctionObject(this, FTAG, Id_toLocaleString, + "toLocaleString", 0, parent); + int version = f.getLanguageVersion(); if (version <= Context.VERSION_1_3 && version != Context.VERSION_DEFAULT) @@ -78,16 +89,81 @@ final class Arguments extends IdScriptableObject @Override public String getClassName() { - return "Object"; + return FTAG; } + @Override + public Object execIdCall(IdFunctionObject f, Context cx, Scriptable scope, + Scriptable thisObj, Object[] args) + { + int methodId = f.methodId(); + switch (methodId) { + case Id_toString: + return getObjectPrototypeMethod("toString"). + call(cx, scope, thisObj, args); + case Id_toLocaleString: + return getObjectPrototypeMethod("toLocaleString"). + call(cx, scope, thisObj, args); + } + throw f.unknown(); + } + + private Callable getObjectPrototypeMethod(String name) { + Scriptable proto = (Scriptable) objectCtor.getPrototypeProperty(); + Object method = proto.get(name, proto); + if ( !(method instanceof Callable) ) { + throw ScriptRuntime.notFunctionError(proto, method, name); + } + return (Callable) method; + } + + private Object arg(int index) { + if (index < 0 || args.length <= index) return NOT_FOUND; + return args[index]; + } + + // the following helper methods assume that 0 < index < args.length + + private void putIntoActivation(int index, Object value) { + String argName = activation.function.getParamOrVarName(index); + activation.put(argName, activation, value); + } + + private Object getFromActivation(int index) { + String argName = activation.function.getParamOrVarName(index); + return activation.get(argName, activation); + } + + private void replaceArg(int index, Object value) { + if (sharedWithActivation(index)) { + putIntoActivation(index, value); + } + synchronized (this) { + if (args == activation.originalArgs) { + args = args.clone(); + } + args[index] = value; + } + } + + private void removeArg(int index) { + synchronized (this) { + if (args[index] != NOT_FOUND) { + if (args == activation.originalArgs) { + args = args.clone(); + } + args[index] = NOT_FOUND; + } + } + } + + // end helpers + @Override public boolean has(int index, Scriptable start) { - if (0 <= index && index < args.length) { - if (args[index] != NOT_FOUND) { - return true; - } + if (arg(index) != NOT_FOUND) { + return true; } return super.has(index, start); } @@ -95,19 +171,16 @@ final class Arguments extends IdScriptableObject @Override public Object get(int index, Scriptable start) { - if (0 <= index && index < args.length) { - Object value = args[index]; - if (value != NOT_FOUND) { - if (sharedWithActivation(index)) { - NativeFunction f = activation.function; - String argName = f.getParamOrVarName(index); - value = activation.get(argName, activation); - if (value == NOT_FOUND) Kit.codeBug(); - } - return value; - } - } + final Object value = arg(index); + if (value == NOT_FOUND) { return super.get(index, start); + } else { + if (sharedWithActivation(index)) { + return getFromActivation(index); + } else { + return value; + } + } } private boolean sharedWithActivation(int index) @@ -133,42 +206,19 @@ final class Arguments extends IdScriptableObject @Override public void put(int index, Scriptable start, Object value) { - if (0 <= index && index < args.length) { - if (args[index] != NOT_FOUND) { - if (sharedWithActivation(index)) { - String argName; - argName = activation.function.getParamOrVarName(index); - activation.put(argName, activation, value); - return; - } - synchronized (this) { - if (args[index] != NOT_FOUND) { - if (args == activation.originalArgs) { - args = args.clone(); - } - args[index] = value; - return; - } - } - } + if (arg(index) == NOT_FOUND) { + super.put(index, start, value); + } else { + replaceArg(index, value); } - super.put(index, start, value); } @Override public void delete(int index) { if (0 <= index && index < args.length) { - synchronized (this) { - if (args[index] != NOT_FOUND) { - if (args == activation.originalArgs) { - args = args.clone(); - } - args[index] = NOT_FOUND; - return; - } - } - } + removeArg(index); + } super.delete(index); } @@ -178,8 +228,11 @@ final class Arguments extends IdScriptableObject Id_callee = 1, Id_length = 2, Id_caller = 3, + Id_constructor = 4, + Id_toString = 5, + Id_toLocaleString = 6, - MAX_INSTANCE_ID = 3; + MAX_INSTANCE_ID = 6; @Override protected int getMaxInstanceId() @@ -191,13 +244,17 @@ final class Arguments extends IdScriptableObject protected int findInstanceIdInfo(String s) { int id; -// #generated# Last update: 2007-05-09 08:15:04 EDT +// #generated# Last update: 2009-08-07 14:12:07 EST L0: { id = 0; String X = null; int c; - if (s.length()==6) { - c=s.charAt(5); + L: switch (s.length()) { + case 6: c=s.charAt(5); if (c=='e') { X="callee";id=Id_callee; } else if (c=='h') { X="length";id=Id_length; } else if (c=='r') { X="caller";id=Id_caller; } + break L; + case 8: X="toString";id=Id_toString; break L; + case 11: X="constructor";id=Id_constructor; break L; + case 14: X="toLocaleString";id=Id_toLocaleString; break L; } if (X!=null && X!=s && !X.equals(s)) id = 0; break L0; @@ -211,6 +268,9 @@ final class Arguments extends IdScriptableObject case Id_callee: case Id_caller: case Id_length: + case Id_constructor: + case Id_toString: + case Id_toLocaleString: attr = DONTENUM; break; default: throw new IllegalStateException(); @@ -227,6 +287,9 @@ final class Arguments extends IdScriptableObject case Id_callee: return "callee"; case Id_length: return "length"; case Id_caller: return "caller"; + case Id_constructor: return "constructor"; + case Id_toString: return "toString"; + case Id_toLocaleString: return "toLocaleString"; } return null; } @@ -248,6 +311,12 @@ final class Arguments extends IdScriptableObject } return value; } + case Id_constructor: + return constructor; + case Id_toString: + return toString; + case Id_toLocaleString: + return toLocaleString; } return super.getInstanceIdValue(id); } @@ -261,6 +330,9 @@ final class Arguments extends IdScriptableObject case Id_caller: callerObj = (value != null) ? value : UniqueTag.NULL_VALUE; return; + case Id_constructor: constructor = value; return; + case Id_toString: toString = value; return; + case Id_toLocaleString: toLocaleString = value; return; } super.setInstanceIdValue(id, value); } @@ -269,17 +341,14 @@ final class Arguments extends IdScriptableObject Object[] getIds(boolean getAll) { Object[] ids = super.getIds(getAll); - if (getAll && args.length != 0) { - boolean[] present = null; + if (args.length != 0) { + boolean[] present = new boolean[args.length]; int extraCount = args.length; for (int i = 0; i != ids.length; ++i) { Object id = ids[i]; if (id instanceof Integer) { int index = ((Integer)id).intValue(); if (0 <= index && index < args.length) { - if (present == null) { - present = new boolean[args.length]; - } if (!present[index]) { present[index] = true; extraCount--; @@ -287,6 +356,14 @@ final class Arguments extends IdScriptableObject } } } + if (!getAll) { // avoid adding args which were redefined to non-enumerable + for (int i = 0; i < present.length; i++) { + if (!present[i] && super.has(i, this)) { + present[i] = true; + extraCount--; + } + } + } if (extraCount != 0) { Object[] tmp = new Object[extraCount + ids.length]; System.arraycopy(ids, 0, tmp, extraCount, ids.length); @@ -304,6 +381,57 @@ final class Arguments extends IdScriptableObject return ids; } + @Override + protected ScriptableObject getOwnPropertyDescriptor(Context cx, Object id) { + double d = ScriptRuntime.toNumber(id); + int index = (int) d; + if (d != index) { + return super.getOwnPropertyDescriptor(cx, id); + } + Object value = arg(index); + if (value == NOT_FOUND) { + return super.getOwnPropertyDescriptor(cx, id); + } + if (sharedWithActivation(index)) { + value = getFromActivation(index); + } + if (super.has(index, this)) { // the descriptor has been redefined + ScriptableObject desc = super.getOwnPropertyDescriptor(cx, id); + desc.put("value", desc, value); + return desc; + } else { + Scriptable scope = getParentScope(); + if (scope == null) scope = this; + return buildDataDescriptor(scope, value, EMPTY); + } + } + + @Override + public void defineOwnProperty(Context cx, Object id, ScriptableObject desc) { + super.defineOwnProperty(cx, id, desc); + + double d = ScriptRuntime.toNumber(id); + int index = (int) d; + if (d != index) return; + + Object value = arg(index); + if (value == NOT_FOUND) return; + + if (isAccessorDescriptor(desc)) { + removeArg(index); + return; + } + + Object newValue = getProperty(desc, "value"); + if (newValue == NOT_FOUND) return; + + replaceArg(index, newValue); + + if (isFalse(getProperty(desc, "writable"))) { + removeArg(index); + } + } + // Fields to hold caller, callee and length properties, // where NOT_FOUND value tags deleted properties. // In addition if callerObj == NULL_VALUE, it tags null for scripts, as @@ -312,9 +440,14 @@ final class Arguments extends IdScriptableObject private Object callerObj; private Object calleeObj; private Object lengthObj; + private Object constructor; + private Object toString; + private Object toLocaleString; private NativeCall activation; + private BaseFunction objectCtor; + // Initially args holds activation.getOriginalArgs(), but any modification // of its elements triggers creation of a copy. If its element holds NOT_FOUND, // it indicates deleted index, in which case super class is queried. diff --git a/mozilla/js/rhino/src/org/mozilla/javascript/ScriptableObject.java b/mozilla/js/rhino/src/org/mozilla/javascript/ScriptableObject.java index fdc03d6a043..c0adea6d775 100644 --- a/mozilla/js/rhino/src/org/mozilla/javascript/ScriptableObject.java +++ b/mozilla/js/rhino/src/org/mozilla/javascript/ScriptableObject.java @@ -1635,9 +1635,13 @@ public abstract class ScriptableObject implements Scriptable, Serializable, } private void defineOwnProperty(Context cx, Slot slot, ScriptableObject desc, int attributes) { + String name = slot.name; + int index = slot.indexOrHash; + if (isAccessorDescriptor(desc)) { - if ( !(slot instanceof GetterSlot) ) - slot = getSlot(cx, slot.name, SLOT_MODIFY_GETTER_SETTER); + if ( !(slot instanceof GetterSlot) ) { + slot = getSlot(cx, (name != null ? name : index), SLOT_MODIFY_GETTER_SETTER); + } GetterSlot gslot = (GetterSlot) slot; @@ -1654,7 +1658,7 @@ public abstract class ScriptableObject implements Scriptable, Serializable, gslot.setAttributes(attributes); } else { if (slot instanceof GetterSlot && isDataDescriptor(desc)) { - slot = getSlot(cx, slot.name, SLOT_CONVERT_ACCESSOR_TO_DATA); + slot = getSlot(cx, (name != null ? name : index), SLOT_CONVERT_ACCESSOR_TO_DATA); } Object value = getProperty(desc, "value"); @@ -1715,11 +1719,11 @@ public abstract class ScriptableObject implements Scriptable, Serializable, } } - private static boolean isTrue(Object value) { + protected static boolean isTrue(Object value) { return (value == NOT_FOUND) ? false : ScriptRuntime.toBoolean(value); } - private static boolean isFalse(Object value) { + protected static boolean isFalse(Object value) { return !isTrue(value); } @@ -1824,6 +1828,10 @@ public abstract class ScriptableObject implements Scriptable, Serializable, return getClassPrototype(scope, "Function"); } + public static Scriptable getArrayPrototype(Scriptable scope) { + return getClassPrototype(scope, "Array"); + } + /** * Get the prototype for the named class. * diff --git a/mozilla/js/rhino/testsrc/doctests/arguments.doctest b/mozilla/js/rhino/testsrc/doctests/arguments.doctest new file mode 100644 index 00000000000..22b2cd2abf3 --- /dev/null +++ b/mozilla/js/rhino/testsrc/doctests/arguments.doctest @@ -0,0 +1,120 @@ +js> var args = (function() { return arguments })() +js> Object.getPrototypeOf(args) === Array.prototype +true +js> args.constructor === Object.prototype.constructor +true +js> Object.getOwnPropertyDescriptor(args, 'constructor').toSource(); +({value:function Object() { [native code for Object.Object, arity=1] } +, writable:true, enumerable:false, configurable:true}) + +js> args.toString() +[object Arguments] + +js> var toStr = Object.prototype.toString; +js> var _ = Object.prototype.toString = function() { + > return this === args ? + > "executes Object.prototype.toString.call(arguments)" : + > "'this' should be 'arguments'" + > } +js> args.toString() +executes Object.prototype.toString.call(arguments) +js> Object.prototype.toString = toStr; undefined + +js> var toLocStr = Object.prototype.toLocaleString; +js> var _ = Object.prototype.toLocaleString = function() { + > return this === args ? + > "executes Object.prototype.toLocaleString.call(arguments)" : + > "'this' should be 'arguments'" + > } +js> args.toLocaleString() +executes Object.prototype.toLocaleString.call(arguments) +js> Object.prototype.toLocaleString = toLocStr; undefined; + +js> (function() { return arguments[2] })('a','b','c') +c +js> (function(x,y,z) { return arguments[2] })('a','b','c') +c + +js> (function(x) { + > arguments[0] = "modified"; + > return x; + > })("original") +modified + +js> (function(x) { + > x = "modified"; + > return arguments[0]; + > })("original") +modified + +js> (function(x) { + > delete arguments[0]; + > arguments[0] = "modified"; + > return x; + > })("original") +original + +js> (function(x) { + > delete x; + > var x = "modified"; + > return arguments[0] + > })("original") +modified + +js> (function(x) { + > Object.defineProperty(arguments, 0, {get:function(){return "modified"}}); + > return arguments[0]; + > })("original") +modified + +js> (function(x) { + > Object.defineProperty(arguments, 0, {get:function(){return "modified"}}); + > return x; + > })("original") +original + +js> (function(x) { + > Object.defineProperty(arguments, 0, {value:"modified"}); + > return arguments[0]; + > })("original") +modified + +js> (function(x) { + > Object.defineProperty(arguments, 0, {value:"modified"}); + > return x; + > })("original") +modified + +js> (function() { for (var i in arguments) print(i); })('a','b','c') +0 +1 +2 + +js> (function() { + > arguments.a = 1; + > Object.defineProperty(arguments, 'b', {value:2, enumerable:true}); + > Object.defineProperty(arguments, 'c', {value:3, enumerable:false}); + > Object.defineProperty(arguments, 0, {enumerable:false}); + > for (var p in arguments) print(p); + > })('hi') +a +b + +js> (function() { return Object.getOwnPropertyDescriptor(arguments, 0) === undefined })() +true + +js> (function() { return Object.getOwnPropertyDescriptor(arguments, 0).toSource(); })("a") +({value:"a", writable:true, enumerable:true, configurable:true}) + +js> (function(x) { + > Object.defineProperty(arguments, 0, {enumerable:false}); + > return Object.getOwnPropertyDescriptor(arguments, 0).enumerable; + > })("original") +false + +js> (function() { + > Object.defineProperty(arguments, 0, {value:2, writable:false}); + > arguments[0] = 3; + > return arguments[0]; + > })(1); +2 diff --git a/mozilla/js/rhino/testsrc/opt-1.tests b/mozilla/js/rhino/testsrc/opt-1.tests index 4782e3ecc86..a3c4a92de11 100644 --- a/mozilla/js/rhino/testsrc/opt-1.tests +++ b/mozilla/js/rhino/testsrc/opt-1.tests @@ -892,7 +892,6 @@ ecma_3/FunExpr/fe-001.js ecma_3/FunExpr/fe-002.js ecma_3/Function/15.3.4.3-1.js ecma_3/Function/15.3.4.4-1.js -ecma_3/Function/arguments-001.js ecma_3/Function/arguments-002.js ecma_3/Function/call-001.js ecma_3/Function/regress-131964.js @@ -1281,8 +1280,6 @@ js1_5/Regress/regress-328012.js js1_5/Regress/regress-328897.js js1_5/Regress/regress-329383.js js1_5/Regress/regress-330951.js -js1_5/Regress/regress-334807-01.js -js1_5/Regress/regress-334807-03.js js1_5/Regress/regress-334807-05.js js1_5/Regress/regress-334807-06.js js1_5/Regress/regress-338307.js diff --git a/mozilla/js/rhino/testsrc/opt0.tests b/mozilla/js/rhino/testsrc/opt0.tests index aa00dc147df..c1614f7cd44 100644 --- a/mozilla/js/rhino/testsrc/opt0.tests +++ b/mozilla/js/rhino/testsrc/opt0.tests @@ -893,7 +893,6 @@ ecma_3/FunExpr/fe-001.js ecma_3/FunExpr/fe-002.js ecma_3/Function/15.3.4.3-1.js ecma_3/Function/15.3.4.4-1.js -ecma_3/Function/arguments-001.js ecma_3/Function/arguments-002.js ecma_3/Function/call-001.js ecma_3/Function/regress-131964.js @@ -1278,8 +1277,6 @@ js1_5/Regress/regress-328012.js js1_5/Regress/regress-328897.js js1_5/Regress/regress-329383.js js1_5/Regress/regress-330951.js -js1_5/Regress/regress-334807-01.js -js1_5/Regress/regress-334807-03.js js1_5/Regress/regress-334807-05.js js1_5/Regress/regress-334807-06.js js1_5/Regress/regress-338307.js diff --git a/mozilla/js/rhino/testsrc/opt9.tests b/mozilla/js/rhino/testsrc/opt9.tests index aa00dc147df..c1614f7cd44 100644 --- a/mozilla/js/rhino/testsrc/opt9.tests +++ b/mozilla/js/rhino/testsrc/opt9.tests @@ -893,7 +893,6 @@ ecma_3/FunExpr/fe-001.js ecma_3/FunExpr/fe-002.js ecma_3/Function/15.3.4.3-1.js ecma_3/Function/15.3.4.4-1.js -ecma_3/Function/arguments-001.js ecma_3/Function/arguments-002.js ecma_3/Function/call-001.js ecma_3/Function/regress-131964.js @@ -1278,8 +1277,6 @@ js1_5/Regress/regress-328012.js js1_5/Regress/regress-328897.js js1_5/Regress/regress-329383.js js1_5/Regress/regress-330951.js -js1_5/Regress/regress-334807-01.js -js1_5/Regress/regress-334807-03.js js1_5/Regress/regress-334807-05.js js1_5/Regress/regress-334807-06.js js1_5/Regress/regress-338307.js