The default bug view has changed. See this FAQ.

Incorrect error message with adjacent "let" blocks

RESOLVED FIXED in mozilla11

Status

()

Core
JavaScript Engine
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: h4writer)

Tracking

(Blocks: 1 bug, {testcase})

Trunk
mozilla11
testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
let (UNUSED = 0) { } let (b = 1) { [][b][2]; }

Result:
  TypeError: [][UNUSED] is undefined
Expected:
  TypeError: [][b] is undefined

Updated

6 years ago
Duplicate of this bug: 522858

Updated

6 years ago
Duplicate of this bug: 528205

Updated

6 years ago
Duplicate of this bug: 622389

Updated

6 years ago
Blocks: 622261
(Assignee)

Comment 4

5 years ago
I found the culprit in the way how the function "GetLocal" in jsopcode.cpp works.
If the function is called from "js_DecompileValueGenerator" it (can) have a negative offset.
Then a search through "script->objects" takes place to find the name of the variable.
Now there is the fault, the code assumes the first object that goes through all tests is the right one. Here in this case we find the first object (containing "UNUSED"), but should find the second one (containing "b").

I adjusted the code. Instead of looping all objects to find the right object. I look at the JSOP_ENTERBLOCK definition (if available) to obtain the index to the right object. Upon fail it will run the old code (and do a loop).

The patch is a bit dirty. I used a goto and there is some duplicate code. But because I don't understand everything I'm afraid to brake something. So the code mimics the old code as good as possible.

I ran the jstests.py testset. I'm not sure if it would detect faults in error reporting?
I couldn't run it totally, because it kept running after xx minutes. So I aborted.
with patch: [3153|   9| 164]
Only timeouts or Date related regressions.
(Assignee)

Comment 5

5 years ago
Created attachment 576830 [details] [diff] [review]
Patch
(Assignee)

Comment 6

5 years ago
(In reply to Hannes Verschore from comment #4)
> ... everything I'm afraid to brake something...
*break

Comment 7

5 years ago
Created attachment 576868 [details] [diff] [review]
Reworked Hannes patch to remove goto

I applied the logic in my tree and reworked the patch to remove the goto:. I confirm the patch does fix the issue. Someone else needs to vet the logic to make sure it is sane (any additional JS_ASSERT()s needed?).

Comment 8

5 years ago
Comment on attachment 576868 [details] [diff] [review]
Reworked Hannes patch to remove goto

Asking dmandelin for review or to reassign to someone who could vet this logic.
Attachment #576868 - Flags: review?(dmandelin)
Comment on attachment 576868 [details] [diff] [review]
Reworked Hannes patch to remove goto

The fix looks OK. Could you please add a test case?
Attachment #576868 - Flags: review?(dmandelin)
(Assignee)

Comment 10

5 years ago
Created attachment 577538 [details] [diff] [review]
Andrew's reworked patch of mine + added testcase
Attachment #576830 - Attachment is obsolete: true
Attachment #576868 - Attachment is obsolete: true
Attachment #577538 - Flags: review?(dmandelin)
(Assignee)

Comment 11

5 years ago
Created attachment 577608 [details] [diff] [review]
Add testcase to jstests.list

Just noticed I forgot to add the testcase to the jstest.list file.
Attachment #577608 - Flags: review?(dmandelin)
Comment on attachment 577538 [details] [diff] [review]
Andrew's reworked patch of mine + added testcase

Thanks!
Attachment #577538 - Flags: review?(dmandelin) → review+
Attachment #577608 - Flags: review?(dmandelin) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed

Updated

5 years ago
Assignee: general → hv1989
http://hg.mozilla.org/integration/mozilla-inbound/rev/b4d21a257883
Flags: in-testsuite+
Keywords: checkin-needed
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/b4d21a257883
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.