Closed Bug 460116 Opened 13 years ago Closed 13 years ago

buggy inCond propagation in js_FoldConstants

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla1.9.1b2

People

(Reporter: brendan, Assigned: brendan)

References

Details

(Keywords: regression, testcase)

Attachments

(1 file, 1 obsolete file)

Attached patch fixes (obsolete) — Splinter Review
Without the patch for this bug (attached), we get:

js> function (){if(typeof(false||undef))throw "fail"}
function () {
    if (typeof undef) {
        throw "fail";
    }
}
js> function (){if((false&&foo)===(true||bar));}
function () {
    if ((false && foo) === (true || bar)) {
    }
}
js> function (){if((baz&&false&&foo)===(bletch||true||bar));}
function () {
    if ((baz && false && foo) === (bletch || true || bar)) {
    }
}

With the patch, we get:

js> function (){if(typeof(false||undef))throw "fail"}
function () {
    if (typeof (false || undef)) {
        throw "fail";
    }
}
js> function (){if((false&&foo)===(true||bar));}
function () {
    if (false === true) {
    }
}
js> function (){if((baz&&false&&foo)===(bletch||true||bar));}
function () {
    if ((baz && false) === (bletch || true)) {
    }
}

I also took the opportunity to fix JSVERSION_ECMA_3 compilation.

/be
Attachment #343285 - Flags: review?(jorendorff)
Comment on attachment 343285 [details] [diff] [review]
fixes

Looks good, r=me.

(I assumed BindDestructuringVar is unchanged.)
Attachment #343285 - Flags: review?(jorendorff) → review+
(In reply to comment #1)
> (From update of attachment 343285 [details] [diff] [review])
> Looks good, r=me.
> 
> (I assumed BindDestructuringVar is unchanged.)

Yes, hg diff strikes again.

Fixed:

http://hg.mozilla.org/tracemonkey/rev/5fe1b014621d

/be
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Folding (false === true) to false, etc., is an exercise left for later -- js_FoldConstants folds only strings and numbers apart from the new || and && Boolish stuff.

/be
Backed out material changes that were causing 10-instruction iloop in testDivision!

http://hg.mozilla.org/tracemonkey/rev/1db033dd276f

/be
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Needed for typeof correctness -- the === and !== stuff was only better folding, except where it was flat wrong! Gotta test with non-boolean boolish values.

/be
Attachment #343285 - Attachment is obsolete: true
Attachment #343316 - Flags: review?(jorendorff)
Comment on attachment 343316 [details] [diff] [review]
the only material change to put back

yep.
Attachment #343316 - Flags: review?(jorendorff) → review+
Fixed for good:

http://hg.mozilla.org/tracemonkey/rev/7b98ea7f523c

/be
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Depends on: 460180
/cvsroot/mozilla/js/tests/js1_5/decompilation/regress-460116-01.js,v  <--  regress-460116-01.js
initial revision: 1.1
/cvsroot/mozilla/js/tests/js1_5/decompilation/regress-460116-02.js,v  <--  regress-460116-02.js
initial revision: 1.1
/cvsroot/mozilla/js/tests/js1_5/decompilation/regress-460116-03.js,v  <--  regress-460116-03.js
initial revision: 1.1

http://hg.mozilla.org/mozilla-central/rev/6e5c848a2183
Flags: in-testsuite+
Flags: in-litmus-
v 1.9.1 tracemonkey. This passes mozilla-central as well.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.