Reinstate or delete the assertions in TokenPos::TokenPos and TokenPos::box

RESOLVED FIXED in mozilla13

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: jorendorff, Assigned: jorendorff)

Tracking

Other Branch
mozilla13
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
I didn't really intend to check them in; they slipped into the patch and slipped past review. But they fail on browser startup.

They should probably pass, so I want to look into it, but I'm commenting them out for now.
https://hg.mozilla.org/mozilla-central/rev/2d649d1cc360
(Assignee)

Comment 2

6 years ago
Created attachment 568792 [details] [diff] [review]
v1
Assignee: general → jorendorff
Attachment #568792 - Flags: review?(luke)

Comment 3

6 years ago
Since you have left some uses of newBinaryOrAppend, could you explain what it is about these particular uses that broke this condition?  It doesn't leap out at me.
(Assignee)

Comment 4

6 years ago
(In reply to Luke Wagner [:luke] from comment #3)
> Since you have left some uses of newBinaryOrAppend, could you explain what
> it is about these particular uses that broke this condition?  It doesn't
> leap out at me.

The assertion is that the lhs and rhs don't overlap. In all of the other calls to newBinaryOrAppend, lhs and rhs represent two different expressions that appear in the source, in the same order, with an operator token in between. So they are guaranteed not to overlap.

(Most of those cases are for mulExpr1, addExpr1, etc. and are straightforward. The oddball is the destructuring case in Parser::variables. There lhs is a destructuring target, which we parse as a primaryExpr and then verify with CheckDestructuring, and rhs is the init-expression. So again, no overlap can occur, and newBinaryOrAppend is fine.)

Comment 5

6 years ago
Comment on attachment 568792 [details] [diff] [review]
v1

Ah, thanks.
Attachment #568792 - Flags: review?(luke) → review+
It think, I needs this for my patch in Bug 715359, unless I do it in some other way.
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2108d51be2e7
Philor backed bug 695922, bug 678086, and bug 719841 out of mozilla-inbound for causing Tp crashes.
https://hg.mozilla.org/mozilla-central/rev/117f2280bd37
(Assignee)

Comment 9

5 years ago
Found the bug. See bug 678086 comment 7.
(Assignee)

Comment 10

5 years ago
Trying again:
https://hg.mozilla.org/integration/mozilla-inbound/rev/25e4854200e9
https://hg.mozilla.org/mozilla-central/rev/25e4854200e9
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.