Replace NULL with nullptr in all C++ files.

Replace NULL with nullptr in all C++ files.

This matches our style guide, and eliminates the current inconsistency, which can
cause (very slight) lost productivity.

This CL *doesn't* replace NULL with nullptr in comments or log statements,
for two reasons.

One, it often sounds more clunky. "If this argument is null..." vs.
"If this argument is nullptr..."

Also, since "nullptr" takes 3 more characters, it messes up line
wrapping.

So, instead, "NULL" is replaced with "null" in comments, now referring
to the abstract concept of nullness rather than the NULL constant.

BUG=webrtc:7147

[Taylor Brandstetter, 2017-02-11 03:51:14]

deadbeef@webrtc.org changed reviewers:
+ tommi@webrtc.org

[Taylor Brandstetter, 2017-02-11 03:51:17]

Tommi: Do you have any objection to this? And should I ask the broader
webrtc-eng group about it?

[Taylor Brandstetter, 2017-02-11 03:52:12]

Description was changed from

==========
Replace NULL with nullptr in all C++ files.

This CL *doesn't* replace NULL with nullptr in comments, for two
reasons.

One, it often sounds more clunky. "If this argument is null..." vs.
"If this argument is nullptr..."

Also, since "nullptr" takes 3 more characters, it messes up line
wrapping.

So, instead, "NULL" is replaced with "null" in comments, now referring
to the abstract concept of nullness rather than the NULL constant.

BUG=webrtc:7147
==========

to

==========
Replace NULL with nullptr in all C++ files.

This matches our style guide, and eliminates the current inconsistency, which
can
cause lost productivity.

This CL *doesn't* replace NULL with nullptr in comments, for two
reasons.

One, it often sounds more clunky. "If this argument is null..." vs.
"If this argument is nullptr..."

Also, since "nullptr" takes 3 more characters, it messes up line
wrapping.

So, instead, "NULL" is replaced with "null" in comments, now referring
to the abstract concept of nullness rather than the NULL constant.

BUG=webrtc:7147
==========

[Taylor Brandstetter, 2017-02-11 03:52:28]

Description was changed from

==========
Replace NULL with nullptr in all C++ files.

This matches our style guide, and eliminates the current inconsistency, which
can
cause lost productivity.

This CL *doesn't* replace NULL with nullptr in comments, for two
reasons.

One, it often sounds more clunky. "If this argument is null..." vs.
"If this argument is nullptr..."

Also, since "nullptr" takes 3 more characters, it messes up line
wrapping.

So, instead, "NULL" is replaced with "null" in comments, now referring
to the abstract concept of nullness rather than the NULL constant.

BUG=webrtc:7147
==========

to

==========
Replace NULL with nullptr in all C++ files.

This matches our style guide, and eliminates the current inconsistency, which
can
cause (very slight) lost productivity.

This CL *doesn't* replace NULL with nullptr in comments, for two
reasons.

One, it often sounds more clunky. "If this argument is null..." vs.
"If this argument is nullptr..."

Also, since "nullptr" takes 3 more characters, it messes up line
wrapping.

So, instead, "NULL" is replaced with "null" in comments, now referring
to the abstract concept of nullness rather than the NULL constant.

BUG=webrtc:7147
==========

[Taylor Brandstetter, 2017-02-11 03:53:36]

The CQ bit was checked by deadbeef@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-11 03:53:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-11 04:08:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-11 04:08:50]

Dry run: Try jobs failed on following builders:
  mac_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/18641)

[Taylor Brandstetter, 2017-02-11 04:24:05]

Description was changed from

==========
Replace NULL with nullptr in all C++ files.

This matches our style guide, and eliminates the current inconsistency, which
can
cause (very slight) lost productivity.

This CL *doesn't* replace NULL with nullptr in comments, for two
reasons.

One, it often sounds more clunky. "If this argument is null..." vs.
"If this argument is nullptr..."

Also, since "nullptr" takes 3 more characters, it messes up line
wrapping.

So, instead, "NULL" is replaced with "null" in comments, now referring
to the abstract concept of nullness rather than the NULL constant.

BUG=webrtc:7147
==========

to

==========
Replace NULL with nullptr in all C++ files.

This matches our style guide, and eliminates the current inconsistency, which
can
cause (very slight) lost productivity.

This CL *doesn't* replace NULL with nullptr in comments or log statements,
for two reasons.

One, it often sounds more clunky. "If this argument is null..." vs.
"If this argument is nullptr..."

Also, since "nullptr" takes 3 more characters, it messes up line
wrapping.

So, instead, "NULL" is replaced with "null" in comments, now referring
to the abstract concept of nullness rather than the NULL constant.

BUG=webrtc:7147
==========

[Taylor Brandstetter, 2017-02-11 04:26:16]

The CQ bit was checked by deadbeef@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-11 04:26:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-11 04:30:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-11 04:30:10]

Dry run: Try jobs failed on following builders:
  linux_libfuzzer_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_libfuzzer_rel/bui...)
  linux_more_configs on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_more_configs/buil...)
  linux_msan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_msan/builds/17593)

[tommi, 2017-02-11 09:55:34]

In general I think that this is a good idea.  I think that your suggestion to
send an email to the mailing list first, is also good and we should give people
a few days to reply since there might be corner cases we're not thinking of.  In
general I'd like to keep usage of nullptr strictly related to C++ code and allow
exceptions on certain API boundaries where documentation suggests that NULL
should be used. More concretely:

* Keep using 'null' in strings so that we don't change log files and avoid
accidentally introduce some sort of JS incompatibility.
* Windows code in particular should be allowed to use NULL. nullptr should be
preferred, but there are cases where doing so, can be confusing or even
incorrect (see below).
* I think we should land this in smaller CLs that are more easily reviewable so
that we don't miss cases like the above.

https://codereview.webrtc.org/2685783014/diff/20001/webrtc/audio/audio_receiv...
File webrtc/audio/audio_receive_stream.cc (right):

https://codereview.webrtc.org/2685783014/diff/20001/webrtc/audio/audio_receiv...
webrtc/audio/audio_receive_stream.cc:54: << (rtcp_send_transport ? "(Transport)"
: "null");
in cases like these, I think we should not change to nullptr.  The reason is
that nullptr to me means C++ whereas this is log related and representation of
nullptr values in JS and upper level languages is typically 'null'.

https://codereview.webrtc.org/2685783014/diff/20001/webrtc/base/event.cc
File webrtc/base/event.cc (left):

https://codereview.webrtc.org/2685783014/diff/20001/webrtc/base/event.cc#oldc...
webrtc/base/event.cc:30: event_handle_ = ::CreateEvent(NULL,                 //
Security attributes.
NULL on Windows is not exactly the same thing as nullptr.  In most cases it is,
but not always.  As an example, take a look at the return value from
timeSetEvent*.  The return type is MMRESULT, which is a number, an identifier if
successful, but NULL if not successful.

For that reason, I think we should be careful in Windows specific code with
blanket replacing NULL with nullptr. Doing so could make the code look
inconsistent with documentation and at worst, cause compilation failures.  In
the particular case we have here (CreateEvent), nullptr is fine and in my
opinion appropriate.

*
https://msdn.microsoft.com/en-us/library/windows/desktop/dd757634(v=vs.85).aspx

[Taylor Brandstetter, 2017-02-11 17:48:56]

I agree the CL should be split up. There could be corner cases where my script
did the wrong thing, so every file should at least be glanced over by someone.

https://codereview.webrtc.org/2685783014/diff/20001/webrtc/audio/audio_receiv...
File webrtc/audio/audio_receive_stream.cc (right):

https://codereview.webrtc.org/2685783014/diff/20001/webrtc/audio/audio_receiv...
webrtc/audio/audio_receive_stream.cc:54: << (rtcp_send_transport ? "(Transport)"
: "null");
On 2017/02/11 09:55:34, tommi (webrtc) wrote:
> in cases like these, I think we should not change to nullptr.  The reason is
> that nullptr to me means C++ whereas this is log related and representation of
> nullptr values in JS and upper level languages is typically 'null'.

I agree; the script I wrote will use "null" in strings and comments, and
"nullptr" in actual code.

https://codereview.webrtc.org/2685783014/diff/20001/webrtc/base/event.cc
File webrtc/base/event.cc (left):

https://codereview.webrtc.org/2685783014/diff/20001/webrtc/base/event.cc#oldc...
webrtc/base/event.cc:30: event_handle_ = ::CreateEvent(NULL,                 //
Security attributes.
On 2017/02/11 09:55:34, tommi (webrtc) wrote:
> NULL on Windows is not exactly the same thing as nullptr.  In most cases it
is,
> but not always.  As an example, take a look at the return value from
> timeSetEvent*.  The return type is MMRESULT, which is a number, an identifier
if
> successful, but NULL if not successful.
> 
> For that reason, I think we should be careful in Windows specific code with
> blanket replacing NULL with nullptr. Doing so could make the code look
> inconsistent with documentation and at worst, cause compilation failures.  In
> the particular case we have here (CreateEvent), nullptr is fine and in my
> opinion appropriate.
> 
> *
>
https://msdn.microsoft.com/en-us/library/windows/desktop/dd757634(v=vs.85).aspx

I think that's one of the advantages of using nullptr; if you use it where you
should have used an integer type, it produces an error. I ran into this in
"audio_device_win.cc" with some APIs that take DWORD_PTRs.

For APIs that accept DWORD_PTR or ULONG_PTRs, should the style be to use "NULL",
or "reinterpret_cast<DWORD_PTR>(nullptr)"? I guess I'll bring this up in my
email to our group