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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

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.

Comment 1

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/2d649d1cc360

Comment 2

[Jason Orendorff [:jorendorff]]

Attachment: v1

Comment 3

[Luke Wagner [:luke]]

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.

Comment 4

[Jason Orendorff [:jorendorff]]

(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

[Luke Wagner [:luke]]

Comment on attachment 568792 [details] [diff] [review]
v1

Ah, thanks.

Comment 6

[Tom S [:evilpie]]

It think, I needs this for my patch in Bug 715359, unless I do it in some other way.

Comment 7

[Jason Orendorff [:jorendorff]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2108d51be2e7

Comment 8

[Joe Drew (not getting mail)]

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

Comment 9

[Jason Orendorff [:jorendorff]]

Found the bug. See bug 678086 comment 7.

Comment 10

[Jason Orendorff [:jorendorff]]

Trying again:
https://hg.mozilla.org/integration/mozilla-inbound/rev/25e4854200e9

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/25e4854200e9