Closed Bug 789633 Opened 8 years ago Closed 8 years ago

GC: Improve naming of AssertRootingUnnecessary

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: terrence, Assigned: terrence)

References

Details

Attachments

(1 file)

Attached patch v0Splinter Review
Steve and I settled on AutoAssertNoGC.  This also includes several fixes to the way we make the assertion to make it easier to mix in to other classes that want to implicitly have this assertion.
Attachment #659418 - Flags: review?(sphink)
Blocks: 789637
Comment on attachment 659418 [details] [diff] [review]
v0

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

::: js/src/gc/Root.h
@@ +16,5 @@
>  #include "js/TemplateLib.h"
> +
> +#if defined(JSGC_ROOT_ANALYSIS) || defined(JSGC_USE_EXACT_ROOTING)
> +# include "jspubtd.h"
> +#endif

I would prefer if this include were unconditional. It's just one more place to update if we change around our #ifdef tangle, and given that jsapi.h already includes it, it's no big deal to include it unnecessarily.

@@ +476,5 @@
>  extern void
>  CheckStackRoots(JSContext *cx);
>  #endif
>  
> +JS_FRIEND_API(bool) IsRelaxRootChecksSet();

This name is a little funky (I first read it as "is this a set of 'relax root checks', whatever those might be?"). But I don't have a better suggestion, probably partly because I don't know what this is used for.
Attachment #659418 - Flags: review?(sphink) → review+
Removed conditional guards and updated the name to NeedRelaxedRootChecks.

Pushed at:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ac83718e89b
https://hg.mozilla.org/mozilla-central/rev/2ac83718e89b
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.