Closed Bug 1154950 Opened 8 years ago Closed 8 years ago

Share our process-global marking paths

Categories

(Core :: JavaScript: GC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: terrence, Assigned: terrence)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

MarkPermanentAtom and MarkWellKnownSymbol are identical except for an assertion. The assertion doesn't seem to be adding much that isn't already asserted elsewhere, so I'd rather have the copy-paste gone than the assertion.
Attachment #8593060 - Flags: review?(sphink)
Comment on attachment 8593060 [details] [diff] [review]
9_share_process_global_tracing_fns-v0.diff

Review of attachment 8593060 [details] [diff] [review]:
-----------------------------------------------------------------

Seems good to me. Just notes on a pre-existing comment.

::: js/src/gc/Marking.cpp
@@ +376,5 @@
> +    JS_ROOT_MARKING_ASSERT(trc);
> +    MOZ_ASSERT(ThingIsPermanentAtomOrWellKnownSymbol(thing));
> +
> +    // We have to mark permanent atoms through a special method because the
> +    // default DoMarking implementation automatically skips them. Fortunatly,

*Fortunately

@@ +377,5 @@
> +    MOZ_ASSERT(ThingIsPermanentAtomOrWellKnownSymbol(thing));
> +
> +    // We have to mark permanent atoms through a special method because the
> +    // default DoMarking implementation automatically skips them. Fortunatly,
> +    // atoms cannot refer to other GC things, so they do not need to go through

symbols and atoms?
Attachment #8593060 - Flags: review?(sphink) → review+
https://hg.mozilla.org/mozilla-central/rev/037b2e086aa6
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.