Closed Bug 886829 Opened 6 years ago Closed 6 years ago

In-source comments incorrectly say JSClass::finalizer is mandatory

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: jonco, Assigned: jonco)

Details

Attachments

(1 file)

While we're at it, we could also assert that the things that are in fact mandatory are not null.
Attached patch Proposed changeSplinter Review
So it turns out this situation doesn't arise at the moment.  Rather than doing an extra check, we can just assert that it doesn't happen and add it later if necessary.
Attachment #767779 - Flags: review?(wmccloskey)
(In reply to Jon Coppeard (:jonco) from comment #1)

Please ignore previous comment, which was intended for a different bug!

As stated originally, change documentation and assert mandatory members.
Comment on attachment 767779 [details] [diff] [review]
Proposed change

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

::: js/src/jsclass.h
@@ +210,2 @@
>                                                                                \
>      /* Optionally non-null members start here. */                             \

"Optionally non-null" sounds a bit odd to my ear. Maybe "These members can be null."?
Attachment #767779 - Flags: review?(wmccloskey) → review+
https://hg.mozilla.org/mozilla-central/rev/68cb015e3e21
https://hg.mozilla.org/mozilla-central/rev/3184e72d56af
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.