Last Comment Bug 695922 - Reinstate or delete the assertions in TokenPos::TokenPos and TokenPos::box
: Reinstate or delete the assertions in TokenPos::TokenPos and TokenPos::box
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: x86 Mac OS X
: -- normal with 1 vote (vote)
: mozilla13
Assigned To: Jason Orendorff [:jorendorff]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-19 16:55 PDT by Jason Orendorff [:jorendorff]
Modified: 2012-02-03 11:42 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (4.83 KB, patch)
2011-10-21 15:12 PDT, Jason Orendorff [:jorendorff]
luke: review+
Details | Diff | Splinter Review

Description Jason Orendorff [:jorendorff] 2011-10-19 16:55:52 PDT
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] 2011-10-20 03:13:23 PDT
https://hg.mozilla.org/mozilla-central/rev/2d649d1cc360
Comment 2 Jason Orendorff [:jorendorff] 2011-10-21 15:12:41 PDT
Created attachment 568792 [details] [diff] [review]
v1
Comment 3 Luke Wagner [:luke] 2011-10-22 07:52:26 PDT
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] 2011-11-14 12:39:33 PST
(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] 2011-11-14 14:49:36 PST
Comment on attachment 568792 [details] [diff] [review]
v1

Ah, thanks.
Comment 6 Tom Schuster [:evilpie] 2012-01-05 03:15:32 PST
It think, I needs this for my patch in Bug 715359, unless I do it in some other way.
Comment 7 Jason Orendorff [:jorendorff] 2012-01-27 12:17:31 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/2108d51be2e7
Comment 8 Joe Drew (not getting mail) 2012-01-28 20:02:00 PST
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] 2012-02-01 14:16:00 PST
Found the bug. See bug 678086 comment 7.
Comment 10 Jason Orendorff [:jorendorff] 2012-02-02 05:30:23 PST
Trying again:
https://hg.mozilla.org/integration/mozilla-inbound/rev/25e4854200e9
Comment 11 Ed Morley [:emorley] 2012-02-03 11:42:33 PST
https://hg.mozilla.org/mozilla-central/rev/25e4854200e9

Note You need to log in before you can comment on or make changes to this bug.