Closed
Bug 737640
Opened 12 years ago
Closed 12 years ago
BaseShape default copy constructor gets used
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: bholley, Assigned: terrence)
Details
Attachments
(1 file, 1 obsolete file)
1.80 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
This confused the crap out of me me while debugging. Patch coming right up.
Reporter | ||
Comment 1•12 years ago
|
||
Attaching a patch - flagging bhackett for review.
Attachment #607723 -
Flags: review?(bhackett1024)
Reporter | ||
Comment 2•12 years ago
|
||
CCing billm, because I think this might have been a GC hazard. Bill, judging from what happens in the other constructors, it looks like we weren't doing write barriers correctly before in the copy constructor case. Do we need write barriers for things that were previously null?
Comment 3•12 years ago
|
||
(In reply to Bobby Holley (:bholley) from comment #2) > Do we need write barriers for things that were previously null? Not for incremental GC, but we will need them for generational, I believe.
(In reply to Andrew McCreight [:mccr8] from comment #3) > (In reply to Bobby Holley (:bholley) from comment #2) > > Do we need write barriers for things that were previously null? > > Not for incremental GC, but we will need them for generational, I believe. Yeah. Given a HeapValue or HeapPtr, you can use the .init() method rather than assignment, and this will skip the incremental write barrier. It will still do the generational barrier.
Comment 5•12 years ago
|
||
Comment on attachment 607723 [details] [diff] [review] Define a copy constructor for BaseShape. v1 The copy constructor for BaseShape should be MOZ_DELETE. Where is it actually used?
Attachment #607723 -
Flags: review?(bhackett1024)
Reporter | ||
Comment 6•12 years ago
|
||
(In reply to Brian Hackett (:bhackett) from comment #5) > Where is it actually used? http://mxr.mozilla.org/mozilla-central/source/js/src/jsscope.cpp#118
Assignee | ||
Comment 7•12 years ago
|
||
Assignee: bobbyholley+bmo → terrence
Attachment #607723 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #623841 -
Flags: review?(bhackett1024)
Updated•12 years ago
|
Attachment #623841 -
Flags: review?(bhackett1024) → review+
Assignee | ||
Comment 8•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf06b5b0a96b
Comment 9•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/bf06b5b0a96b
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in
before you can comment on or make changes to this bug.
Description
•