Closed Bug 1316432 Opened 3 years ago Closed 3 years ago

Make nsCOMPtr constructible&assignable from nullptr

Categories

(Core :: XPCOM, defect)

49 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: gerald, Assigned: gerald)

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 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 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 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+
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
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
Depends on: 1317368
You need to log in before you can comment on or make changes to this bug.