Disallow C++11 compilers from constructing OwnPtrs from 0

Disallow C++11 compilers from constructing OwnPtrs from 0

C++11 allows 0 to be implicit converted to std::nullptr_t, but Blink's
shim for std::nullptr_t does not have such support. To avoid inconsistency
between native and shimmed versions, delete the int constructor, which
makes conversions from 0 fail properly in C++11 mode.

R=jamesr
BUG=254668
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=153846

[jamesr, 2013-06-26 23:13:24]

Hmm, while I like consistency I think implicitly converting '0' is not really a
great thing to support.  Why would we want to do that instead of nullptr?

[adamk, 2013-06-26 23:32:13]

On 2013/06/26 23:13:24, jamesr wrote:
> Hmm, while I like consistency I think implicitly converting '0' is not really
a
> great thing to support.  Why would we want to do that instead of nullptr?

The trouble is that C++11 will support that, as long as we're using
std::nullptr_t: C++11 implicitly converts 0 to a std::nullptr_t.

If we don't want to allow 0 in these cases, we need something else, say a new
BlinkNullPtr.

[jamesr, 2013-06-26 23:34:42]

On 2013/06/26 23:32:13, adamk wrote:
> On 2013/06/26 23:13:24, jamesr wrote:
> > Hmm, while I like consistency I think implicitly converting '0' is not
really
> a
> > great thing to support.  Why would we want to do that instead of nullptr?
> 
> The trouble is that C++11 will support that, as long as we're using
> std::nullptr_t: C++11 implicitly converts 0 to a std::nullptr_t.

Yes, but if we have a more specific override for int it'll prefer that, won't
it?  See the bug for an alternate proposal.  It should only try to go 0 ->
std::nullptr_t if there's no better match.
> 
> If we don't want to allow 0 in these cases, we need something else, say a new
> BlinkNullPtr.

[adamk, 2013-07-09 21:34:28]

I've updated the patch to delete the int constructor, as requested.

I feel a little bad about scattering #if COMPILER_SUPPORTS stuff around, so if
you'd prefer I delete these in the classic way I can. But the advantage here is
better error messages.

[jamesr, 2013-07-09 21:49:33]

I think this is great.  Which toolchains have you tested this with?

[adamk, 2013-07-09 21:55:44]

On 2013/07/09 21:49:33, jamesr wrote:
> I think this is great.  Which toolchains have you tested this with?

Only gcc on Precise (4.6.3, it seems) and clang on Linux so far. Firing off try
jobs now.

The toolchain that'd be problematic is one that supports std::nullptr_t natively
but not function deletion. I don't know if we have such a toolchain (seems
unlikely?).

[jyasskin (not chrome), 2013-07-09 21:58:11]

On Tue, Jul 9, 2013 at 2:55 PM,  <adamk@chromium.org> wrote:
> The toolchain that'd be problematic is one that supports std::nullptr_t
> natively
> but not function deletion. I don't know if we have such a toolchain (seems
> unlikely?).

http://msdn.microsoft.com/en-us/library/vstudio/hh567368.aspx says
MSVC 10 and 11 are such toolchains.

[adamk, 2013-07-09 22:09:01]

Per jyasskin, updated to delete the constructor via a macro.

This still results in a nice error message in clang:

../../third_party/WebKit/Source/core/dom/MutationObserverInterestGroup.cpp:47:16:
error: conversion function from 'int' to
'PassOwnPtr<WebCore::MutationObserverInterestGroup>' invokes a deleted function
        return 0;
               ^
../../third_party/WebKit/Source/wtf/PassOwnPtr.h:43:45: note: function has been
explicitly marked deleted here
        WTF_DISALLOW_CONSTRUCTION_FROM_ZERO(PassOwnPtr);
                                            ^
../../third_party/WebKit/Source/wtf/NullPtr.h:59:9: note: expanded from macro
'WTF_DISALLOW_CONSTRUCTION_FROM_ZERO'
        ClassName(int) = delete
        ^
1 error generated.

[jamesr, 2013-07-09 22:16:29]

lgtm. watch the trybots and bots with care

[commit-bot: I haz the power, 2013-07-09 22:45:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adamk@chromium.org/17959002/11001

[commit-bot: I haz the power, 2013-07-10 00:45:34]

Sorry for I got bad news for ya.
Compile failed with a clobber build on mac_layout.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_layout...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-07-10 01:03:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adamk@chromium.org/17959002/11001

[commit-bot: I haz the power, 2013-07-10 04:23:19]

Change committed as 153846