Closed Bug 1238786 Opened 9 years ago Closed 9 years ago

Make public tracing functions allow nullable pointers by default

Categories

(Core :: JavaScript: GC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: terrence, Assigned: terrence)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

When I moved everything over to JS::TraceEdge<T> and friends last week, I kept the same null-handling semantics as we currently expose. I'd like to change these to always null-check to simplify callers, but put it off to a later change in the off chance that one of these callers is actually hot on some benchmark we care about. I don't think that's too likely though, so let's make it happen. https://treeherder.mozilla.org/#/jobs?repo=try&revision=f5188f30e186
Attachment #8706667 - Flags: review?(sphink)
Comment on attachment 8706667 [details] [diff] [review] 1_implicit_nullable_public_tracing-sm-v0.diff Review of attachment 8706667 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/public/TracingAPI.h @@ +295,5 @@ > // is used as a key in a hashtable, then the object must be removed and > // re-inserted with the correct hash. > +// > +// Note that while |edgep| must never be null, it fine to call this on nullable > +// pointers without checking for nullptr first. "it fine" Though I wonder if it reads better as: Note that while |edgep| must never be null, it is fine for *edgep to be nullptr.
Attachment #8706667 - Flags: review?(sphink) → review+
Attachment #8706668 - Flags: review?(bugs) → review+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: