1316432 - Make nsCOMPtr constructible&assignable from nullptr

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

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 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1316432 - Replace 0's with nullptr in nsCOMPtr.* -

Just a mechanical find/replace of all zero pointers, before the next patch.

Review commit: https://reviewboard.mozilla.org/r/91974/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/91974/

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1316432 - nsCOMPtr construction/assignment from nullptr -

Review commit: https://reviewboard.mozilla.org/r/91976/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/91976/

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1316432 - Fix nsCOMPtr constructions&assignments from 0 -

Giving '0' (literal zero) to nsCOMPtr is now ambiguous, as both
nsCOMPtr(decltype(nullptr)) and nsCOMPtr(T*) could be used.
In any case, our coding standards mandate the use of 'nullptr' for pointers.
So I'm changing all zeroes into nullptr's where necessary.

Review commit: https://reviewboard.mozilla.org/r/91978/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/91978/

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ee472a3f1d9573c381ecb8c147249465ea80bc60

Comment 5

[Nathan Froyd [:froydnj]]

Comment on attachment 8809354 [details]
Bug 1316432 - Fix nsCOMPtr constructions&assignments from 0 -

https://reviewboard.mozilla.org/r/91978/#review91960

Comment 6

[Nathan Froyd [:froydnj]]

Comment on attachment 8809352 [details]
Bug 1316432 - Replace 0's with nullptr in nsCOMPtr.* -

https://reviewboard.mozilla.org/r/91974/#review91962

Comment 7

[Nathan Froyd [:froydnj]]

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`.

Comment 8

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

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 9

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8809352 [details]
Bug 1316432 - Replace 0's with nullptr in nsCOMPtr.* -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/91974/diff/1-2/

Comment 10

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8809353 [details]
Bug 1316432 - nsCOMPtr construction/assignment from nullptr -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/91976/diff/1-2/

Comment 11

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8809354 [details]
Bug 1316432 - Fix nsCOMPtr constructions&assignments from 0 -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/91978/diff/1-2/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
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

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/09a8134e26ea
https://hg.mozilla.org/mozilla-central/rev/376b38e78518
https://hg.mozilla.org/mozilla-central/rev/8d3dcf1db881