Closed
Bug 76634
Opened 24 years ago
Closed 24 years ago
Has Function.prototype.toString() changed?
Categories
(Core :: JavaScript Engine, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla0.9
People
(Reporter: pschwartau, Assigned: brendan)
Details
(Keywords: js1.5)
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
These testcases are erroring because Function.prototype.toString()
is not behaving as it used to:
js1_2/function/Function_object.js
js1_2/function/tostring-1.js
js1_4/Regress/function-002.js
For example, the first two tests fail because of this:
js> var f = new Function()
js> f.toString()
(function () {
})
The testcases expect
function () {
}
Meanwhile, js1_4/Regress/function-002.js fails because of this:
js> print(f2.toString())
function f2() {
var y;
f1(1, 2), y = (function g(x) {return Math.exp(x);});
print(y.toString());
}
The testcase expects no line breaks:
js> print(dec2)
function f2(){var y; f1(1,2), y=function g(x){return Math.exp(x);};
print(y.toString())}
(Of course, Bugzilla is wrapping this text -)
Reporter | ||
Comment 1•24 years ago
|
||
Also note the extra spaces being inserted in f2.toString() above:
e.g. f1(1, 2)
vs. f1(1,2)
and y = (function g(x) etc.
vs. y=function g(x) etc.
Assignee | ||
Comment 2•24 years ago
|
||
Argh. I broke ECMA-262 15.3.4.2 in trying to fix bug 73760. This is a
standards conformance regression that adds noise to the js/tests suite, so I'd
like to fix for 0.9. Patch coming up.
/be
Assignee: rogerl → brendan
Severity: normal → critical
Keywords: js1.5,
mozilla0.9
Priority: -- → P1
Target Milestone: --- → mozilla0.9
Assignee | ||
Comment 3•24 years ago
|
||
phil, the decompiler has *always* put spaces on either side of =, and after
commas in arg lists. Since 1995, so I don't think the test should expect to get
back whatever white space style it used in its source.
/be
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•24 years ago
|
||
Assignee | ||
Comment 5•24 years ago
|
||
I was running the testsuite these past few days, but I had convinced myself that
ECMA allowed enough latitude in Function.prototype.toString that the tests were
wrong and the change checked in to fix bug 73760 were right. But ECMA requires
an implementation-dependent string that parses as a FunctionDeclaration, and the
parentheses break that.
The patch distinguishes toSource from toString down in the bowels of the
decompiler, by testing for the don't-pretty-print flag. Shaver loves my old
decompiler, so he gets sr= duties. Beard, if rogerl is not around, can you r=
today, for 0.9? Thanks,
/be
Boy, I can't get enough of that decompiler. sr=shaver. (But a comment in the
code to indicate why you're checking ->pretty would be very helpful the next
time someone other than you has to wade into that code.)
Reporter | ||
Comment 7•24 years ago
|
||
Brendan: thank you for pointing out the whitespace behavior. On closer
examination of the js1_4 testcase, I see it actually compares these:
StripSpaces(f2.toString()) == StripSpaces(dec2));
where StripSpaces() strips out spaces, carriage returns, etc.
Sorry about that -
Assignee | ||
Comment 8•24 years ago
|
||
Reporter | ||
Comment 9•24 years ago
|
||
With patch id=31445 , the two js1_2 testcases no longer error.
But the js1_4 testcase still fails:
[//d/JS_EXP/mozilla/js/src/WINNT4.0_DBG.OBJ]
$ ./js
js> load('../../tests/js1_4/shell.js')
js> load('../../tests/js1_4/Regress/function-002.js')
BUGNUMBER: 330462
function-002.js Regression test case for 325843
typeof f1 = function PASSED!
f1.toString() == dec1 = true PASSED!
typeof f2 = function PASSED!
f2.toString() == dec2 = false FAILED! expected: true
js> StripSpaces(f2.toString())
functionf2(){varyf1(1,2),y=(functiong(x){returnMath.exp(x)})print(y.toString())}
js> StripSpaces(dec2)
functionf2(){varyf1(1,2),y=functiong(x){returnMath.exp(x)}print(y.toString())}
Assignee | ||
Comment 10•24 years ago
|
||
Phil: that test is requiring something that ECMA-262 Edition 3 15.3.42. does
not: that the outer function's toString use Function.prototype.toString on the
inner function when decompiling or otherwise recovering the source. There is no
requirement. The decompilation of f2 is free to parenthesize lambda expressions
such as the one assigned to var y, named function g.
You'll need to tweak that test, somehow. Short of making it allow for parens
around nested function expressions, what should it do? What is it really trying
to test that's not covered by other tests?
/be
Reporter | ||
Comment 11•24 years ago
|
||
The body of that outer function is just something I made up to replace:
dec2 = "function f2(){1,2}";
I was just writing anything that had some complexity. I will tweak the test -
sr=shaver (again), thanks.
Comment 13•24 years ago
|
||
r=beard
Comment 14•24 years ago
|
||
a=asa for checkin to 0.9
Assignee | ||
Comment 15•24 years ago
|
||
Fix is in.
/be
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 16•24 years ago
|
||
Testcase js/tests/js1_4/Regress/function-002.js has been corrected.
The testcases are now passing on Linux and WinNT. Marking Verified.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•