Note: There are a few cases of duplicates in user autocompletion which are being worked on.

BaseShape default copy constructor gets used

RESOLVED FIXED in mozilla15

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bholley, Assigned: terrence)

Tracking

unspecified
mozilla15
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

This confused the crap out of me me while debugging. Patch coming right up.
Created attachment 607723 [details] [diff] [review]
Define a copy constructor for BaseShape. v1

Attaching a patch - flagging bhackett for review.
Attachment #607723 - Flags: review?(bhackett1024)
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?
(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 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)
(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

5 years ago
Created attachment 623841 [details] [diff] [review]
v1: Go through StackBaseShape.
Assignee: bobbyholley+bmo → terrence
Attachment #607723 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #623841 - Flags: review?(bhackett1024)
Attachment #623841 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 8

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf06b5b0a96b

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/bf06b5b0a96b
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.