Closed
Bug 1316432
Opened 8 years ago
Closed 8 years ago
Make nsCOMPtr constructible&assignable from nullptr
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: mozbugz, Assigned: mozbugz)
References
Details
Attachments
(3 files)
Similarly to bug 1316206, which made RefPtr work with nullptr, we should do the same with nsCOMPtr. Repeating the expected benefits: - It would appear as a separate call in stack traces. - Slight optimization, as there would be no need to check for a non-null pointer that would need an AddRef(). - It will enforce the Mozilla coding style that requires the use of 'nullptr' for pointers in C++.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•8 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ee472a3f1d9573c381ecb8c147249465ea80bc60
Comment 5•8 years ago
|
||
mozreview-review |
Comment on attachment 8809354 [details] Bug 1316432 - Fix nsCOMPtr constructions&assignments from 0 - https://reviewboard.mozilla.org/r/91978/#review91960
Attachment #8809354 -
Flags: review?(nfroyd) → review+
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8809352 [details] Bug 1316432 - Replace 0's with nullptr in nsCOMPtr.* - https://reviewboard.mozilla.org/r/91974/#review91962
Attachment #8809352 -
Flags: review?(nfroyd) → review+
Comment 7•8 years ago
|
||
mozreview-review |
Comment on attachment 8809353 [details] Bug 1316432 - nsCOMPtr construction/assignment from nullptr - https://reviewboard.mozilla.org/r/91976/#review91964 r=me with or without addressing the comment below. ::: xpcom/glue/nsCOMPtr.h:435 (Diff revision 1) > + MOZ_IMPLICIT nsCOMPtr(std::nullptr_t) > + : NSCAP_CTOR_BASE(nullptr) With the `RefPtr` patches, you went with `decltype(nullptr)` for these overloads; was there a particular reason for the switch here? I see the `::operator==` overloads for `nsCOMPtr` use `decltype(nullptr)`, and I guess I have a slight (like epsilon-slight) preference for using that instead of `std::nullptr_t`.
Attachment #8809353 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 8•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8809353 [details] Bug 1316432 - nsCOMPtr construction/assignment from nullptr - https://reviewboard.mozilla.org/r/91976/#review91964 > With the `RefPtr` patches, you went with `decltype(nullptr)` for these overloads; was there a particular reason for the switch here? I see the `::operator==` overloads for `nsCOMPtr` use `decltype(nullptr)`, and I guess I have a slight (like epsilon-slight) preference for using that instead of `std::nullptr_t`. Oh dear, I thought there was no 'decltype(nullptr)' in nsCOMPtr.h, that's why I used the more std:: route! Ok I'll revert to decltype to be more consistent in the file and with RefPtr. Should we make this a Coding Style rule, so it's the same everywhere? Doing a quick foxsearch, at the moment the score is: nullptr_t 45 vs decltype(nullptr) 79
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 12•8 years ago
|
||
Pushed by gsquelart@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/09a8134e26ea Replace 0's with nullptr in nsCOMPtr.* - r=froydnj https://hg.mozilla.org/integration/autoland/rev/376b38e78518 nsCOMPtr construction/assignment from nullptr - r=froydnj https://hg.mozilla.org/integration/autoland/rev/8d3dcf1db881 Fix nsCOMPtr constructions&assignments from 0 - r=froydnj
Comment 13•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/09a8134e26ea https://hg.mozilla.org/mozilla-central/rev/376b38e78518 https://hg.mozilla.org/mozilla-central/rev/8d3dcf1db881
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•