Closed Bug 140974 Opened 21 years ago Closed 21 years ago

JS constant folder over-eager with if (false) var x=42

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.0

People

(Reporter: brendan, Assigned: brendan)

References

Details

(Keywords: js1.5)

Attachments

(1 file, 1 obsolete file)

deal with a for (var ...;;) ... counterexample (for-loop head is ternary)
1.72 KB, patch
khanson
: review+
shaver
: superreview+
asa
: approval+
Details | Diff | Splinter Review 
ECMA correctness, would like to fix for 1.0.

/be
Status: NEW → ASSIGNED
Keywords: js1.5, mozilla1.0
Target Milestone: --- → mozilla1.0
Attached patch proposed fix (obsolete) — — Splinter Review 
Looking for fast testing and review (worksforme, looks right).

/be
Thanks to shaver for pointing this out!

/be
Attachment #81535 - Attachment is obsolete: true
Comment on attachment 81536 [details] [diff] [review]
deal with a for (var ...;;) ... counterexample (for-loop head is ternary)

Cool. sr=shaver.
Attachment #81536 - Flags: superreview+
Not to worry, I've nuked that break; after return ...; in the PN_TERNARY case of
the latest patch.

/be
The latest patch passes the JS testsuite on WinNT and Linux, in both
the debug and optimized shell. I got only the known test failures.

I will add Martin's test to the testsuite later today -
Comment on attachment 81536 [details] [diff] [review]
deal with a for (var ...;;) ... counterexample (for-loop head is ternary)

r=khanson
Attachment #81536 - Flags: review+
Checked into trunk.  Going for 1.0 approval, it's an easy ECMA-purity fix.

/be
Blocks: 138000
Comment on attachment 81536 [details] [diff] [review]
deal with a for (var ...;;) ... counterexample (for-loop head is ternary)

a=asa (on behalf of drivers) for checkin to the 1.0 branch
Attachment #81536 - Flags: approval+
Fixed in trunk and branch.

/be
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
adding branch resolution keyword.
Keywords: fixed1.0.0
Testcase added to JS testsuite:

            mozilla/js/tests/js1_5/Regress/regress-140974.js

Sections 4 through 6 of this test failed before the fix.
The test now passes on the trunk and on the 1.0.0 branch.

Marking Verified FIXED and adding verified1.0.0 keyword -
Status: RESOLVED → VERIFIED
Keywords: verified1.0.0
Typo:        'Sections 4 through 6 of this test failed before the fix'
should be:   'Sections 3 through 6 of this test failed before the fix'
Flags: testcase+
You need to log in before you can comment on or make changes to this bug.