Decompiler emits extra parens around assignment in "for(;;)" condition

RESOLVED FIXED

Status

()

defect
--
minor
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: jruderman, Assigned: jorendorff)

Tracking

({regression, testcase})

Trunk
x86
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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?
Summary: Decompiler omits extra parens around assignment in "for(;;)" condition → Decompiler emits extra parens around assignment in "for(;;)" condition
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.
Posted patch fixSplinter Review
Assignee: general → jorendorff
Attachment #344318 - Flags: review?(brendan)
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+
Probably because it's common to test for equality in "if" conditions, but "for" conditions are usually inequalities and occasionally assignments.
http://hg.mozilla.org/tracemonkey/rev/e2ab86422d2f
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
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.