Closed
Bug 38384
Opened 25 years ago
Closed 25 years ago
Instance variable is treated as class variable
Categories
(Rhino Graveyard :: Core, defect, P3)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: gerrit.riessen, Assigned: rogerl)
Details
----------- file: nesting_test.js -----------------
function Nest()
{
this.toString_return_value = "";
this.contents = new Array();
this.addElement =
new Function( "elem", "this.contents[this.contents.length] = elem; return
(this);" );
this.toString = NEST_toString_m;
}
function NEST_toString_m()
{
this.toString_return_value = "";
for ( var i = 0; i < this.contents.length; i++ )
{
this.toString_return_value += this.contents[ i ].toString() ;
}
return ( this.toString_return_value );
}
net = new Nest();
net.addElement( 'foobar' );
net.addElement( new Nest().addElement( 'hello world' ) );
net.addElement( 'snafu' );
---------------------------------------------------
when doing a net.toString() in the Rhino shell i get:
js> load( 'nesting_test.js' )
js> net.toString();
hello world
js>
but when i use IE(5.X) to test the code i get, what i think assume
to be correct: 'foobarhello worldsnafu' [same result using Netscape
4.X]
------------ file: nesting_test.html ---------------
<html>
<head>
<title></title>
<script language="JavaScript" src="nesting_test.js"></script>
</head>
<body onLoad="alert( net.toString() )">
</body>
</html>
----------------------------------------------------
I don't know what the ECMAScript Standard says, but i would assume
that the IE/Netscape result is the correct result?
Hope it helps,
Gerrit
|
||
Comment 1•25 years ago
|
||
Proposed fix:
Index: Interpreter.java
===================================================================
RCS file: /m/pub/mozilla/js/rhino/org/mozilla/javascript/Interpreter.java,v
retrieving revision 1.30
diff -u -r1.30 Interpreter.java
--- Interpreter.java 2000/03/13 18:27:25 1.30
+++ Interpreter.java 2000/05/07 05:45:50
@@ -1314,6 +1314,7 @@
byte[] iCode = theData.itsICode;
int pc = 0;
int iCodeLength = theData.itsICodeTop;
+ Scriptable itsThisObj = theData.itsThisObj;
Object[] local = null; // used for newtemp/usetemp etc.
if (theData.itsMaxLocals > 0)
@@ -1659,8 +1660,7 @@
lhs = stack[stackTop];
stack[stackTop] = ScriptRuntime.callSpecial(
cx, lhs, rhs, outArgs,
- theData.itsThisObj,
- scope, name, i);
+ itsThisObj, scope, name, i);
pc += 6;
break;
case TokenStream.CALL :
@@ -1765,7 +1765,7 @@
stack[++stackTop] = null;
break;
case TokenStream.THIS :
- stack[++stackTop] = theData.itsThisObj;
+ stack[++stackTop] = itsThisObj;
break;
case TokenStream.FALSE :
stack[++stackTop] = Boolean.FALSE;
The problem is that the InterpreterData object is referenced for the current
'this' object, but recursive calls can overwrite that value. Copying it into a
local prevents the problem.
This seems a bit inefficient. Roger, do you remember why it was done this way?
Should we just add another parameter to the interpret method?
If the change looks okay to you, Roger, could you check it in? I'll wait for my
new email address to come before I request checkin privileges.
Assignee: norrisboyd → rogerl
|
Assignee | |
Comment 2•25 years ago
|
||
I think the only reason I didn't want it passed around was that it uses up a
slot and a low-index Java register. The testing I did with various JITs
indicated that they were favoring the parameters and earlier-declared variables
(for register allocation? - seems kind of hard to believe on x86, but who
knows). I checked in your fix, but moved the local 'itsThisObj' further down in
the declaration order as it doesn't get used that freqeuntly.
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•