Closed
Bug 461108
Opened 16 years ago
Closed 16 years ago
Decompiler emits extra parens around assignment in "for(;;)" condition
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: jorendorff)
References
Details
(Keywords: regression, testcase)
Attachments
(1 file)
970 bytes,
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
The patch in bug 460501 (removing JSOP_GROUP) made the decompiler put extra parens around assignment in "if", "while", and "for" conditions. Producing extra parens makes sense for "if" and "while" conditions, since naked assignment there triggers a strict warning, "test for equality (==) mistyped as assignment (=)?". But I don't think the extra parens help in "for" conditions, which don't have this warning. Here's a pattern I use frequently: for (i = 0; attr = attrs[i]; ++i) {} The decompiler now turns it into something I find less readable: for (i = 0; (attr = attrs[i]); ++i) {} Can we go back to leaving parens out here, while leaving them in for if/while?
Reporter | ||
Updated•16 years ago
|
Summary: Decompiler omits extra parens around assignment in "for(;;)" condition → Decompiler emits extra parens around assignment in "for(;;)" condition
Assignee | ||
Comment 1•16 years ago
|
||
OK. It won't be a lot of code to revert this. I'm surprised we warn about this in "while" conditions and not "for" conditions, though.
Assignee | ||
Comment 2•16 years ago
|
||
Assignee: general → jorendorff
Attachment #344318 -
Flags: review?(brendan)
Comment 3•16 years ago
|
||
Comment on attachment 344318 [details] [diff] [review] fix I'm not sure why for lacks that strict warning, probably just an oversight. But it's not worth changing at this point. /be
Attachment #344318 -
Flags: review?(brendan) → review+
Reporter | ||
Comment 4•16 years ago
|
||
Probably because it's common to test for equality in "if" conditions, but "for" conditions are usually inequalities and occasionally assignments.
Assignee | ||
Comment 5•16 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/e2ab86422d2f
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 6•16 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/37b3fdbb0f07 /cvsroot/mozilla/js/tests/js1_5/decompilation/regress-461108.js,v <-- regress-461108.js initial revision: 1.1
Flags: in-testsuite+
Flags: in-litmus-
You need to log in
before you can comment on or make changes to this bug.
Description
•