Closed Bug 460870 Opened 13 years ago Closed 13 years ago

Round-trip change with RHS of ||

Categories

(Core :: JavaScript Engine, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9.1b2

People

(Reporter: jruderman, Assigned: brendan)

References

Details

(Keywords: testcase)

Attachments

(1 file)

js> f = (function() { if (a || 1 || 2) { } })
function () {
    if (a || 1) {
    }
}

js> eval(uneval(f))
function () {
    if (a || true) {
    }
}

Why does the third operand disappear but not the second?  And why does it take until the second round for the second operand to be coerced to boolean during constant folding?
Insufficient work folding conditions, in short. Taking.

/be
Assignee: general → brendan
Status: NEW → ASSIGNED
OS: Mac OS X → All
Priority: -- → P2
Hardware: PC → All
Target Milestone: --- → mozilla1.9.1b2
If the regressing patch was for a followup bug to bug 443074 please correct the dependency. Thanks,

/be
Blocks: 443074
Attached patch fixSplinter Review
Attachment #345362 - Flags: review?(jorendorff)
Attachment #345362 - Flags: review?(jorendorff) → review+
Comment on attachment 345362 [details] [diff] [review]
fix

Jesse asks why we don't simplify to 'if (a) {}'.  That would be a valid simplification; we just don't do it.  (js_FoldConstants does try to detect if the condition of an if-statement or ?: is always true or always false.)
Correction, as brendan points out on IRC, 'if (a) {...}' is not a valid simplification of 'if (a||true) {...}'!
Oops :)  It is a valid simplification of 'if (a||false) {...}', though, right?
Jesse: yes, but as I said on IRC, I'm weary of dancing with the fuzzer and have other fish to fry! File a bug for general@js.bugs if you like.

/be
Fixed on m-c now too:

http://hg.mozilla.org/mozilla-central/rev/0c60665e8150

/be
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
test landed http://hg.mozilla.org/mozilla-central/rev/9f309ed83015 and cvs
Flags: in-testsuite+
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.