Closed Bug 1159806 Opened 9 years ago Closed 9 years ago

Clean up ZoneIsAtomZoneForString and the checking MACROS in Marking.cpp

Categories

(Core :: JavaScript: GC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: terrence, Assigned: terrence)

Details

Attachments

(1 file)

      No description provided.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b2366d59678c

As requested.
Assignee: nobody → terrence
Status: NEW → ASSIGNED
Attachment #8600479 - Flags: review?(sphink)
Comment on attachment 8600479 [details] [diff] [review]
16_zoneismarkingassertion-v0.diff

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

::: js/src/gc/Marking.cpp
@@ +307,5 @@
>  {
> +#ifdef DEBUG
> +    Zone* zone = TenuredCell::fromPointer(thing)->zone();
> +    JSGCTraceKind kind = TenuredCell::fromPointer(thing)->getTraceKind();
> +    bool isStringOrSymbol = (kind == JSTRACE_STRING || kind == JSTRACE_SYMBOL);

er... I thought it would be possible to do

static void ZoneIsMarkingAssertion(Cell* thing)
{
  MOZ_ASSERT(TenuredCell::fromPointer(thing)->zone()->isGCMarking());
}

static void ZoneIsMarkingAssertion(JSString* thing)
{
  zone = ...;
  rt = ...;
  MOZ_ASSERT(zone->isGCMarking() || rt->isAtomsZone(zone));
}

static void ZoneIsMarkingAssertion(Symbol* thing)
{
  zone = ...;
  rt = ...;
  MOZ_ASSERT(zone->isGCMarking() || rt->isAtomsZone(zone));
}

or common those up by making the latter two call a common ZoneIsMarkingOrAtomsAssertion(Cell*).

Also, can we make the name imperative -- AssertZoneIsMarking?

@@ +315,4 @@
>  }
>  
> +static void
> +RootMarkingAssertion(JSTracer* trc)

I'm not sure what this would be called, then. AssertRootMarkingState?
Attachment #8600479 - Flags: review?(sphink)
Oooh, that's much better! Done!
https://hg.mozilla.org/mozilla-central/rev/2e67954d9762
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.