Closed Bug 1023686 Opened 5 years ago Closed 5 years ago

Parsing non-hoisted vars results in wrong opcode when initializer present

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: shu, Unassigned)

References

Details

Attachments

(1 file)

No description provided.
Blocks: 1023609
Could you explain this change and also add a testcase that is broken without it?
(In reply to Luke Wagner [:luke] from comment #2)
> Could you explain this change and also add a testcase that is broken without
> it?

This change is that when parsing non-hoisted lets, we delay define()ing them until the pushLetScope. When that happens, we then go through all the definitions, and define() fixes up their JSOps. This fixing up wasn't taking into account whether the definition node's JSOp was a SET or a GET, and unconditionally changed them to GETs.

As for a testcase, I don't think this code gets exercised currently. I ran into when attempting to remove let expressions and let statements. Currently we desugar

for (let foo = 0;;) stmt

into

let (foo = 0) { for (;;) stmt }

After the removal of let expressions, I was attempting to desugar into

{ let foo = 0; for (;;) stmt }

The for init part is not parsed hoisted, and was getting compiled into something like

zero
getlocal 0

instead of

zero
setlocal 0

Of course the new desugaring is still wrong w.r.t. spec, but that's a separate bug.
Comment on attachment 8438120 [details] [diff] [review]
Fix JSOP of definition nodes of non-hoisted declarations.

Ah, thanks for explaining.
Attachment #8438120 - Flags: review?(luke) → review+
https://hg.mozilla.org/mozilla-central/rev/47bf0d6847e4
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.