C++11 Feature Proposal: Allowing/Encouraging use of nullptr

[Ryan Sleevi]

What:

Allowing/Encouraging the use of nullptr (as opposed to NULL)

Why:

In C++ < 11, NULL may be an integral type (0) or a pointer type (void*)0, although the world is messy and full of lies and disappointment.

In C++11, nullptr is a distinguishable type, which can be used, for example, in template specializations to distinguish between integers, pointer types, and explicit nullptr.

With nullptr support, one could update scoped_ptr<> to handle implicit initialization from nullptr, so that the following code:

scoped_ptr<Foo> GetFoo() {
  return scoped_ptr<Foo>();
}

becomes

scoped_ptr<Foo> GetFoo() {
  return nullptr;
}

Where:

nullptr emulation is already in heavy use within Blink, but its use is not allowed beyond there because of issues with ODR and emulating nullptr in Chromium.

[Mike Frysinger]

On Tue, Sep 23, 2014 at 4:05 PM, Ryan Sleevi <rsl...@chromium.org> wrote:

Where:

nullptr emulation is already in heavy use within Blink, but its use is not allowed beyond there because of issues with ODR and emulating nullptr in Chromium.

CrOS has also adopted it in its platform

-mike

[James Robinson]

On Tue, Sep 23, 2014 at 2:05 PM, Ryan Sleevi <rsl...@chromium.org> wrote:

What:

Allowing/Encouraging the use of nullptr (as opposed to NULL)

Why:

In C++ < 11, NULL may be an integral type (0) or a pointer type (void*)0, although the world is messy and full of lies and disappointment.

In C++11, nullptr is a distinguishable type, which can be used, for example, in template specializations to distinguish between integers, pointer types, and explicit nullptr.

With nullptr support, one could update scoped_ptr<> to handle implicit initialization from nullptr, so that the following code:

scoped_ptr<Foo> GetFoo() {
  return scoped_ptr<Foo>();
}

becomes

scoped_ptr<Foo> GetFoo() {
  return nullptr;
}

I strongly support this proposal, but haven't figured out how to make this work with our C++03 move emulation code for scoped_ptr<T>.  Folks smarter than me are pretty sure this can be done but I haven't seen it yet.  Could you link to a patch that works for this and move emulation?

Also be aware that we can't use std::nullptr_t yet, since that's a library feature, but we could allow decltype(nullptr) (which is the same thing).

- James

[Ryan Sleevi]

On Tue, Sep 23, 2014 at 2:13 PM, James Robinson <jam...@chromium.org> wrote:

On Tue, Sep 23, 2014 at 2:05 PM, Ryan Sleevi <rsl...@chromium.org> wrote:

What:

Allowing/Encouraging the use of nullptr (as opposed to NULL)

Why:

In C++ < 11, NULL may be an integral type (0) or a pointer type (void*)0, although the world is messy and full of lies and disappointment.

In C++11, nullptr is a distinguishable type, which can be used, for example, in template specializations to distinguish between integers, pointer types, and explicit nullptr.

With nullptr support, one could update scoped_ptr<> to handle implicit initialization from nullptr, so that the following code:

scoped_ptr<Foo> GetFoo() {
  return scoped_ptr<Foo>();
}

becomes

scoped_ptr<Foo> GetFoo() {
  return nullptr;
}

I strongly support this proposal, but haven't figured out how to make this work with our C++03 move emulation code for scoped_ptr<T>.  Folks smarter than me are pretty sure this can be done but I haven't seen it yet.  Could you link to a patch that works for this and move emulation?

I had a patch somewhere that did this within the constraints of our C++03 code, but would have required base::nullptr, which was the kiss of death for it. It's potentially buried in my email somewhere, but may have been deleted. I'm sure someone smarter than me on codereview could find it, since I had uploaded it for discussion.

But yes, it is indeed possible.

[Dana Jansens]

On Tue, Sep 23, 2014 at 5:24 PM, Ryan Sleevi <rsl...@chromium.org> wrote:

On Tue, Sep 23, 2014 at 2:13 PM, James Robinson <jam...@chromium.org> wrote:

On Tue, Sep 23, 2014 at 2:05 PM, Ryan Sleevi <rsl...@chromium.org> wrote:

What:

Allowing/Encouraging the use of nullptr (as opposed to NULL)

Why:

In C++ < 11, NULL may be an integral type (0) or a pointer type (void*)0, although the world is messy and full of lies and disappointment.

In C++11, nullptr is a distinguishable type, which can be used, for example, in template specializations to distinguish between integers, pointer types, and explicit nullptr.

With nullptr support, one could update scoped_ptr<> to handle implicit initialization from nullptr, so that the following code:

scoped_ptr<Foo> GetFoo() {
  return scoped_ptr<Foo>();
}

becomes

scoped_ptr<Foo> GetFoo() {
  return nullptr;
}

I strongly support this proposal, but haven't figured out how to make this work with our C++03 move emulation code for scoped_ptr<T>.  Folks smarter than me are pretty sure this can be done but I haven't seen it yet.  Could you link to a patch that works for this and move emulation?

I had a patch somewhere that did this within the constraints of our C++03 code, but would have required base::nullptr, which was the kiss of death for it. It's potentially buried in my email somewhere, but may have been deleted. I'm sure someone smarter than me on codereview could find it, since I had uploaded it for discussion.

But yes, it is indeed possible.

I believe it's here https://codereview.chromium.org/11411318/

 

 

Also be aware that we can't use std::nullptr_t yet, since that's a library feature, but we could allow decltype(nullptr) (which is the same thing).

- James

 

Where:

nullptr emulation is already in heavy use within Blink, but its use is not allowed beyond there because of issues with ODR and emulating nullptr in Chromium.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Ryan Sleevi]

On Tue, Sep 23, 2014 at 2:36 PM, Dana Jansens <dan...@chromium.org> wrote:

On Tue, Sep 23, 2014 at 5:24 PM, Ryan Sleevi <rsl...@chromium.org> wrote:

On Tue, Sep 23, 2014 at 2:13 PM, James Robinson <jam...@chromium.org> wrote:

On Tue, Sep 23, 2014 at 2:05 PM, Ryan Sleevi <rsl...@chromium.org> wrote:

What:

Allowing/Encouraging the use of nullptr (as opposed to NULL)

Why:

In C++ < 11, NULL may be an integral type (0) or a pointer type (void*)0, although the world is messy and full of lies and disappointment.

In C++11, nullptr is a distinguishable type, which can be used, for example, in template specializations to distinguish between integers, pointer types, and explicit nullptr.

With nullptr support, one could update scoped_ptr<> to handle implicit initialization from nullptr, so that the following code:

scoped_ptr<Foo> GetFoo() {
  return scoped_ptr<Foo>();
}

becomes

scoped_ptr<Foo> GetFoo() {
  return nullptr;
}

I strongly support this proposal, but haven't figured out how to make this work with our C++03 move emulation code for scoped_ptr<T>.  Folks smarter than me are pretty sure this can be done but I haven't seen it yet.  Could you link to a patch that works for this and move emulation?

I had a patch somewhere that did this within the constraints of our C++03 code, but would have required base::nullptr, which was the kiss of death for it. It's potentially buried in my email somewhere, but may have been deleted. I'm sure someone smarter than me on codereview could find it, since I had uploaded it for discussion.

But yes, it is indeed possible.

I believe it's here https://codereview.chromium.org/11411318/

Indeed, that's the one! With ubiquitous nullptr support, it'd also be a lot less complicated (note that some of the overhead came from the nullptr emulation)

[Albert J. Wong (王重傑)]

Adding to the chorus of YES.

[James Robinson]

On Tue, Sep 23, 2014 at 2:47 PM, Ryan Sleevi <rsl...@chromium.org> wrote:

Indeed, that's the one! With ubiquitous nullptr support, it'd also be a lot less complicated (note that some of the overhead came from the nullptr emulation)

OK, we've got ubiquitous nullptr support.  I think if someone links to a patch that adds it to base/scoped_ptr.h and compiles we can approve it.  First person to post a working patch gets a gold star in my imaginary scrapbook.

Are there situations other than scoped_ptr where we want to use nullptr? In your original post you said:

"In C++11, nullptr is a distinguishable type, which can be used, for example, in template specializations to distinguish between integers, pointer types, and explicit nullptr."

which is not entirely accurate - std::nullptr_t is a type, but nullptr is a keyword.  We don't have std::nullptr_t yet since it's a library feature.  It is equivalent to decltype(nullptr) which is a pure language construct and thus something we could use today, but decltype() in general is tricky and I don't think we are ready to allow it across the board.  Do you have some examples of other situations where it would be useful to do this sort of specialization?

- James

[Ryan Sleevi]

From the vague annals of memory, it's come up in a few other places where we rely on template specializations based on type traits or SFINAE. I'm thinking of T*/U* distinguishability, operator overloading of comparators for our RAII types (scoped_ptr, scoped_refptr), and things like CallbackTraits.

For the user-visible effects, I mostly just anticipate this being one less hurdle when converting code to take/receive a scoped_ptr<T> over a T*, since typing scoped_ptr<T>() is annoying. 

[Nico Weber]

For the user-visible effects, I mostly just anticipate this being one less hurdle when converting code to take/receive a scoped_ptr<T> over a T*, since typing scoped_ptr<T>() is annoying. 

Another visible benefit is that nullptr is guaranteed to be pointer-sized, so if you for example call a varargs function that gets its arguments terminated by NULL / 0 / nullptr, then nullptr guarantees that all 8 bytes of the last parameter are zero. With NULL, this is usually the case, but it's not guaranteed. 

[Matt Mueller]

Is it helpful for EXPECT_EQ, CHECK_EQ, etc? I seem to remember there are problems using NULL with those.

[Peter Kasting]

On Tue, Sep 23, 2014 at 3:49 PM, Matt Mueller <ma...@chromium.org> wrote:

Is it helpful for EXPECT_EQ, CHECK_EQ, etc? I seem to remember there are problems using NULL with those.

Yes; due to the way those templates work, you generally can't do:

EXPECT_EQ(NULL, foo());

You have to do either:

EXPECT_TRUE(foo() == NULL);

or the heinous:

EXPECT_EQ(static_cast<Foo*>(NULL), foo());

I believe that nullptr eliminates this issue but I haven't tested it.

PK

[James Robinson]

This is where patches are helpful :).  Here's the comment from base/logging.h:

// WARNING: These may not compile correctly if one of the arguments is a pointer

// and the other is NULL. To work around this, simply static_cast NULL to the

// type of the desired pointer.

and here's some code that uses it:

base/logging_unittest.cc:196:  DCHECK_EQ(mock_log_source.Log(), static_cast<const char*>(NULL))

Could you get rid of the need for the static_cast<const char*> there?  Probably - send us a patch showing how!

- James

On Tue, Sep 23, 2014 at 3:49 PM, Matt Mueller <ma...@chromium.org> wrote:

[Ryan Sleevi]

On Tue, Sep 23, 2014 at 3:52 PM, James Robinson <jam...@chromium.org> wrote:

This is where patches are helpful :).  Here's the comment from base/logging.h:

// WARNING: These may not compile correctly if one of the arguments is a pointer

// and the other is NULL. To work around this, simply static_cast NULL to the

// type of the desired pointer.

and here's some code that uses it:

base/logging_unittest.cc:196:  DCHECK_EQ(mock_log_source.Log(), static_cast<const char*>(NULL))

Could you get rid of the need for the static_cast<const char*> there?  Probably - send us a patch showing how!

- James

Indeed, you could get rid of it.

However, gtest is in the list of build_glibc/build_newlib that Nico pointed out, so isn't a patch pointless (but demonstrative)? 

[Nico Weber]

gtest's source itself can't use c++11. Libraries using gtest can.

[Nico Weber]

On Tue, Sep 23, 2014 at 3:51 PM, Peter Kasting <pkas...@chromium.org> wrote:

On Tue, Sep 23, 2014 at 3:49 PM, Matt Mueller <ma...@chromium.org> wrote:

Is it helpful for EXPECT_EQ, CHECK_EQ, etc? I seem to remember there are problems using NULL with those.

Yes; due to the way those templates work, you generally can't do:

EXPECT_EQ(NULL, foo());

Small correction: gtest explicitly makes EXPECT_EQ(NULL, ...) work ( https://code.google.com/p/googletest/wiki/FAQ#Why_does_Google_Test_support_EXPECT_EQ(NULL,_ptr)_and_ASSERT_EQ( ), but it doesn't work for e.g. EXPECT_NE. nullptr does seem to make this work.

[Peter Kasting]

On Tue, Sep 23, 2014 at 3:55 PM, Nico Weber <tha...@chromium.org> wrote:

gtest's source itself can't use c++11. Libraries using gtest can.

Incidentally, I think upstream gtest and gmock are actually C++11 at this point -- I tried to bump us to the latest versions a few months ago and the compile broke due to them using C++11 features.  I've been waiting on the C++11 OK to be able to rev these.  I forget if they use library features though.

PK

[Nico Weber]

On Tue, Sep 23, 2014 at 3:57 PM, Peter Kasting <pkas...@chromium.org> wrote:

On Tue, Sep 23, 2014 at 3:55 PM, Nico Weber <tha...@chromium.org> wrote:

gtest's source itself can't use c++11. Libraries using gtest can.

Incidentally, I think upstream gtest and gmock are actually C++11 at this point -- I tried to bump us to the latest versions a few months ago and the compile broke due to them using C++11 features.  I've been waiting on the C++11 OK to be able to rev these.

ncbray@ bumped it recently; he asked them to add a "don't use c++11 features yet" define, due to the nacl dep.

[Alex Vakulenko]

We are already using EXPECT_NE/EXPECT_EQ with nullptr in ChromeOS core and it works. If I remember correctly, CHECK_EQ/CHECK_NE still doesn't.

--

[James Robinson]

For gtest, correct.  However the bit of code I referenced (the DCHECK/CHECK family) is in base/logging.h which is not in the set that needs to continue supporting C++03.

- James

[Hendrik]

+1 -- any chance we can also sweep out the existing NULLs while we're at it?

[Antoine Labour]

On Tue, Sep 23, 2014 at 2:13 PM, James Robinson <jam...@chromium.org> wrote:

On Tue, Sep 23, 2014 at 2:05 PM, Ryan Sleevi <rsl...@chromium.org> wrote:

What:

Allowing/Encouraging the use of nullptr (as opposed to NULL)

Why:

In C++ < 11, NULL may be an integral type (0) or a pointer type (void*)0, although the world is messy and full of lies and disappointment.

In C++11, nullptr is a distinguishable type, which can be used, for example, in template specializations to distinguish between integers, pointer types, and explicit nullptr.

With nullptr support, one could update scoped_ptr<> to handle implicit initialization from nullptr, so that the following code:

scoped_ptr<Foo> GetFoo() {
  return scoped_ptr<Foo>();
}

becomes

scoped_ptr<Foo> GetFoo() {
  return nullptr;
}

I strongly support this proposal, but haven't figured out how to make this work with our C++03 move emulation code for scoped_ptr<T>.  Folks smarter than me are pretty sure this can be done but I haven't seen it yet.  Could you link to a patch that works for this and move emulation?

Also be aware that we can't use std::nullptr_t yet, since that's a library feature, but we could allow decltype(nullptr) (which is the same thing).

Can it make sense to have a typedef decltype(nullptr) nullptr_t; in the (say) base namespace, until we can use std::nullptr_t?

Antoine

- James

 

Where:

nullptr emulation is already in heavy use within Blink, but its use is not allowed beyond there because of issues with ODR and emulating nullptr in Chromium.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

[Ryan Sleevi]

On Tue, Sep 23, 2014 at 5:09 PM, Antoine Labour <pi...@google.com> wrote:

On Tue, Sep 23, 2014 at 2:13 PM, James Robinson <jam...@chromium.org> wrote:

On Tue, Sep 23, 2014 at 2:05 PM, Ryan Sleevi <rsl...@chromium.org> wrote:

What:

Allowing/Encouraging the use of nullptr (as opposed to NULL)

Why:

In C++ < 11, NULL may be an integral type (0) or a pointer type (void*)0, although the world is messy and full of lies and disappointment.

In C++11, nullptr is a distinguishable type, which can be used, for example, in template specializations to distinguish between integers, pointer types, and explicit nullptr.

With nullptr support, one could update scoped_ptr<> to handle implicit initialization from nullptr, so that the following code:

scoped_ptr<Foo> GetFoo() {
  return scoped_ptr<Foo>();
}

becomes

scoped_ptr<Foo> GetFoo() {
  return nullptr;
}

I strongly support this proposal, but haven't figured out how to make this work with our C++03 move emulation code for scoped_ptr<T>.  Folks smarter than me are pretty sure this can be done but I haven't seen it yet.  Could you link to a patch that works for this and move emulation?

Also be aware that we can't use std::nullptr_t yet, since that's a library feature, but we could allow decltype(nullptr) (which is the same thing).

Can it make sense to have a typedef decltype(nullptr) nullptr_t; in the (say) base namespace, until we can use std::nullptr_t?

Antoine

The only reason we should need the nullptr_t type would be for template disambiguation.

This assumes that decltype(nullptr) is legal (although not necessarily any decltype), in part, because it's what we've been doing in Blink for some time (which decltype's into... std::nullptr_t :'( )

[Nico Weber]

Can it make sense to have a typedef decltype(nullptr) nullptr_t; in the (say) base namespace, until we can use std::nullptr_t?

If we need nullptr_t in a few places, I think this makes sense. 

[James Robinson]

On Tue, Sep 23, 2014 at 5:14 PM, Ryan Sleevi <rsl...@chromium.org> wrote:

On Tue, Sep 23, 2014 at 5:09 PM, Antoine Labour <pi...@google.com> wrote:

Can it make sense to have a typedef decltype(nullptr) nullptr_t; in the (say) base namespace, until we can use std::nullptr_t?

We could, but I'm not sure how often it would be used and having a base::nullptr_t typedef could be even more confusing.  I think we need to look at the set of use cases for this to decide if a base:: typedef or just saying decltype(nullptr) is cleaner.  I'm having trouble imagining a tremendously large number of uses of base::nullptr_t.  In blink, std::nullptr_t is only used in 14 files, including the ones that define it.  If we need it lots of times, we can add a base:: version.  If we only need it a few times in some base types - say just in logging.h, scoped_ptr and (maybe) weak_ptr - it's probably not worth the overhead of another type.

 

Antoine

The only reason we should need the nullptr_t type would be for template disambiguation.

This assumes that decltype(nullptr) is legal (although not necessarily any decltype), in part, because it's what we've been doing in Blink for some time (which decltype's into... std::nullptr_t :'( )

decltype(nullptr) is legal syntax.  Blink's emulation is conditional so it only compiles the std::nullptr_t in for toolchains that do not have library support for it.  It's technically undefined behavior to declare a type in std in this way, although it's safe so far, but the fact that blink declares the typedef in std means that we can't do the same outside of Blink without generating ODR violations.

- James

[Ryan Sleevi]

Sorry, I meant "legal" in the sense of "An allowed exception to the decltype caveats earlier in this thread".

Put differently, even if we ban decltype() in general (because there might be toolchain issues, which was what was alluded to), decltype(nullptr) would be a limited exception. I also anticipate it being extremely limited, and something that, if it absolutely needed to be a type, would probably be something in base::internal rather than base::

[Mounir Lamouri]

+blin...@chromium.org

I assume that the decision would apply to Chromium and Blink, right?

[James Robinson]

Blink is already using nullptr extensively via emulation. This would have relatively small impact on most of the Blink codebase except for in places where the WTF emulation can't be used, such as the Blink public c++ api, where the impact will probably be fairly small.

- James

[Nico Weber]

Since folks from all time zones had a chance to chime in, I'll attempt a summary:

* Nobody's opposed to using nullptr. It has a few advantages over NULL (works in gtest macros, works better with varargs). We should encourage using nullptr instead of NULL for new code.

* We may or may not add a base::nullptr_t, it probably won't be needed often.

* We won't mass-replace NULL with nullptr (only one person asked for this, it makes blame output harder to parse, and lines containing NULL are often tricky and interesting in blame output).

rsleevi, do you want to send a CL to change src/styleguide/c++/c++11.html to move nullptr to the explicitly allowed bucket?

[Nico Weber]

This is now live: http://chromium-cpp.appspot.com/

[Peter Kasting]

On Wed, Sep 24, 2014 at 12:04 PM, Nico Weber <tha...@chromium.org> wrote:

* We won't mass-replace NULL with nullptr (only one person asked for this, it makes blame output harder to parse, and lines containing NULL are often tricky and interesting in blame output).

Actually I'd like this too.  There are going to be other cases (e.g. the OVERRIDE transformation) where we're going to make mass changes to the codebase, and I think we should always optimize for consistency and clarity and never for blame -- it's not as if people can't click through one additional level to get blame output, and those clickthroughs gradually taper off over time as the code continues to be modified or becomes less important to spelunk back through time.

I suspect my vote won't sway you, but all I can do is try.

PK

[Nico Weber]

(Another issue that I just learned about that makes this difficult is that the Windows code uses NULL heavily in places where no pointer is expected – several of the HANDLE types (not HANDLE itself I believe, but e.g. TRACEHANDLE), and MSDN even says to use NULL for those (http://msdn.microsoft.com/en-us/library/windows/desktop/aa363686(v=vs.85).aspx). This would probably make a mass find-and-replace somewhat involved too.)

[Peter Kasting]

On Wed, Sep 24, 2014 at 2:06 PM, Nico Weber <tha...@chromium.org> wrote:

(Another issue that I just learned about that makes this difficult is that the Windows code uses NULL heavily in places where no pointer is expected – several of the HANDLE types (not HANDLE itself I believe, but e.g. TRACEHANDLE), and MSDN even says to use NULL for those (http://msdn.microsoft.com/en-us/library/windows/desktop/aa363686(v=vs.85).aspx). This would probably make a mass find-and-replace somewhat involved too.)

If we're doing that in our code, that would definitely complicate any mass replacement.  That's a good point.

That said, I suspect the number of real cases we'd break would be small and manageable.  The use of Windows types is significantly more limited than just "all Windows-specific code", and the number of those cases that this applies to is, I hope, small.

PK

[Chris Hopman]

I think I agree with Peter. As we introduce new features, particularly when we are going to say that you should use the new way and not the old way, I think we should prefer to mass rewrite things to use the new way, even if it comes at some cost in churn/blameability. That being said, for cases where the conversion is complicated, then just a having a policy of converting code as it is changed/introduced/etc may be fine.

In this case, maybe we can have a clang plugin/reformatter that identifies places where NULL is actually used as a pointer (like when it is immediately implicitly converted)... and maybe also cases where it is used as a sentinel vararg :). This wouldn't get all cases, but would probably get most while still being reasonable safe.

--

[Adrienne Walker]

"Optimize for consistency and clarity and never for blame" should be a
Chromium style rule.

2014-09-24 14:02 GMT-07:00 'Peter Kasting' via blink-dev
<blin...@chromium.org>:

2014-09-24 14:02 GMT-07:00 'Peter Kasting' via blink-dev
<blin...@chromium.org>:

[Peter Kasting]

On Wed, Sep 24, 2014 at 2:15 PM, Chris Hopman <cjho...@chromium.org> wrote:

In this case, maybe we can have a clang plugin/reformatter that identifies places where NULL is actually used as a pointer (like when it is immediately implicitly converted)... and maybe also cases where it is used as a sentinel vararg :). This wouldn't get all cases, but would probably get most while still being reasonable safe.

Even changing NULL -> nullptr in cross-platform code and leaving Windows devs to update code as they see fit would be OK with me.  I don't want to let perfect be the enemy of good here.

PK

[Chris Hopman]

+1, if our only concerns are the Windows issues nico mentioned, then yeah, that's even better. 

PK

[James Robinson]

If we can do a machine conversion I think we should, but I'm not entirely sure this will be easy.  I don't want to block usage of the feature on their being a good rewriter.  For example, we'll have to reformat many lines of code since nullptr is 3 characters longer which might be tricky in parts of the codebase that aren't currently clang-formatted.

How about we allow nullptr, encourage conversion, and if somebody cooks up a workable translator we allow using it but we do not make that a condition of use of the feature?

- James

On Wed, Sep 24, 2014 at 2:15 PM, Chris Hopman <cjho...@chromium.org> wrote:

[Hendrik]

TRACEHANDLE is ULONG64, so that one won't compile after the replace. Besides, we should probably replace it with =0 instead.  I did a quick look, there looks to only be a few places where it needs to be fixed, so a mass replace, compile, and fix should be fine for this one.

As you mentioned, HANDLE will be fine to use nullptr, as it's just a void*

I suspect the other window handle types would fall into one of these two categories.

[Hendrik]

Lets change the character limit to 83 :)

Just kidding, that's a good point!

On Wednesday, September 24, 2014 2:33:47 PM UTC-7, James Robinson wrote:

If we can do a machine conversion I think we should, but I'm not entirely sure this will be easy.  I don't want to block usage of the feature on their being a good rewriter.  For example, we'll have to reformat many lines of code since nullptr is 3 characters longer which might be tricky in parts of the codebase that aren't currently clang-formatted.

How about we allow nullptr, encourage conversion, and if somebody cooks up a workable translator we allow using it but we do not make that a condition of use of the feature?

- James

On Wed, Sep 24, 2014 at 2:15 PM, Chris Hopman wrote:

I think I agree with Peter. As we introduce new features, particularly when we are going to say that you should use the new way and not the old way, I think we should prefer to mass rewrite things to use the new way, even if it comes at some cost in churn/blameability. That being said, for cases where the conversion is complicated, then just a having a policy of converting code as it is changed/introduced/etc may be fine.

In this case, maybe we can have a clang plugin/reformatter that identifies places where NULL is actually used as a pointer (like when it is immediately implicitly converted)... and maybe also cases where it is used as a sentinel vararg :). This wouldn't get all cases, but would probably get most while still being reasonable safe.

On Wed, Sep 24, 2014 at 2:02 PM, 'Peter Kasting' via Chromium-dev wrote:

[Nico Weber]

On Wed, Sep 24, 2014 at 2:36 PM, Hendrik <hend...@chromium.org> wrote:

TRACEHANDLE is ULONG64, so that one won't compile after the replace. Besides, we should probably replace it with =0 instead.  I did a quick look, there looks to only be a few places where it needs to be fixed, so a mass replace, compile, and fix should be fine for this one.

TRACEHANDLE was just one example, the same is true for various other handle types. I was looking at this for some unrelated reason, and there's hundreds of build warnings about conversions like this ( http://llvm.org/bugs/show_bug.cgi?id=12146#c5 ).

It sounds like everyone but me is in favor of rewriting NULLs to nullptrs, so if someone finds a good way of doing that, we should probably do it. New code can just use it, starting today.

[Peter Kasting]

On Wed, Sep 24, 2014 at 2:33 PM, James Robinson <jam...@chromium.org> wrote:

If we can do a machine conversion I think we should, but I'm not entirely sure this will be easy.  I don't want to block usage of the feature on their being a good rewriter.  For example, we'll have to reformat many lines of code since nullptr is 3 characters longer which might be tricky in parts of the codebase that aren't currently clang-formatted.

clang-format only reformats lines you touch, right?  So presumably using it to reformat these lines would still have minimal impact on the rest of these files (it wouldn't clang-format the whole codebase).

How about we allow nullptr, encourage conversion, and if somebody cooks up a workable translator we allow using it but we do not make that a condition of use of the feature?

I agree that we shouldn't gate on this.  Conversion is a nice-to-have.

My hope was that someone like Daniel who knows enough to do something like the override conversion would be able to handle this as well.

PK 

[Sam McNally]

clang-modernize has a transform for this: http://clang.llvm.org/extra/UseNullptrTransform.html

--

[Peter Kasting]

On Wed, Sep 24, 2014 at 6:01 PM, Sam McNally <sa...@chromium.org> wrote:

clang-modernize has a transform for this: http://clang.llvm.org/extra/UseNullptrTransform.html

Awesome.  Anyone want to volunteer to try and run this on parts of Chromium?

PK 

[Daniel Cheng]

Note the default clang checkout doesn't include the extra clang tools anymore. I added support for it in https://codereview.chromium.org/12213081, but later removed it because no one was using it anymore. Since then, we've moved from make to CMake for building clang so my original patch will likely need to be updated.

Daniel

[Sam McNally]

I gave clang-modernize a try on src/extensions/ and https://codereview.chromium.org/598173003/ is the result.