Bug 66024 – nullptr can't be used for PassRefPtr

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[James Robinson]

nullptr can't be used for PassRefPtr

[James Robinson]

Created attachment 103561 [details]
Patch

[James Robinson]

When a function defined to return PassRefPtr<T> wants to return a null value, it currently has to do either:

  return 0;

which isn't as descriptive as it could be or

  return PassRefPtr<T>();

which can be a lot of typing if "T" is long.  It'd be nice to be able to do

  return nullptr;

like you can for PassOwnPtr<>.

It looks like this functionality was added by Darin back in http://trac.webkit.org/changeset/69970 but then deleted by Anders in http://trac.webkit.org/changeset/89283.  I think the nullptr constructor was just an innocent victim in http://trac.webkit.org/changeset/89283 - it seems like something that we do want to support.

[Anders Carlsson]

(In reply to comment #2)
> When a function defined to return PassRefPtr<T> wants to return a null value, it currently has to do either:
> 
>   return 0;
> 
> which isn't as descriptive as it could be or
> 
>   return PassRefPtr<T>();
> 
> which can be a lot of typing if "T" is long.  It'd be nice to be able to do
> 
>   return nullptr;
> 
> like you can for PassOwnPtr<>.
> 

Yeah, I agree.

> It looks like this functionality was added by Darin back in http://trac.webkit.org/changeset/69970 but then deleted by Anders in http://trac.webkit.org/changeset/89283.  I think the nullptr constructor was just an innocent victim in http://trac.webkit.org/changeset/89283 - it seems like something that we do want to support.

Looking at the commit, it looks like only the nullptr assignment operator was removed and that there was never a nullptr constructor (unless I'm missing something).

[James Robinson]

(In reply to comment #3)
> (In reply to comment #2)
> > When a function defined to return PassRefPtr<T> wants to return a null value, it currently has to do either:
> > 
> >   return 0;
> > 
> > which isn't as descriptive as it could be or
> > 
> >   return PassRefPtr<T>();
> > 
> > which can be a lot of typing if "T" is long.  It'd be nice to be able to do
> > 
> >   return nullptr;
> > 
> > like you can for PassOwnPtr<>.
> > 
> 
> Yeah, I agree.
> 
> > It looks like this functionality was added by Darin back in http://trac.webkit.org/changeset/69970 but then deleted by Anders in http://trac.webkit.org/changeset/89283.  I think the nullptr constructor was just an innocent victim in http://trac.webkit.org/changeset/89283 - it seems like something that we do want to support.
> 
> Looking at the commit, it looks like only the nullptr assignment operator was removed and that there was never a nullptr constructor (unless I'm missing something).


Aha, you are correct.

Still, I think this patch is good.

[Anders Carlsson]

(In reply to comment #4)
> Aha, you are correct.
> 
> Still, I think this patch is good.

I agree. I'm not sure if it'll break the C++0x build, but seeing as we don't have a C++0x build, I don't think we need to worry about that now.

[WebKit Review Bot]

Comment on attachment 103561 [details]
Patch

Clearing flags on attachment: 103561

Committed r92880: <http://trac.webkit.org/changeset/92880>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Kenichi Ishibashi]

Hi,

This looks great, but the change causes a bunch of compile error when I build Chromium Win port with VS2010.  How can I fix the error?

[James Robinson]

Can you show me a link to a failing build or post the compile errors here?

[Kenichi Ishibashi]

(In reply to comment #9)
> Can you show me a link to a failing build or post the compile errors here?

My build environment is Japanese so you might not get the error description, but the error code is C2668.
http://msdn.microsoft.com/en-us/library/da60x087(v=VS.100).aspx

c:\cygwin\home\bashi\chrome\src\third_party\webkit\source\javascriptcore\wtf\text\StringImpl.h(173): error C2668: 'WTF::PassRefPtr<T>::PassRefPtr' : オーバーロード関数の呼び出しを解決することができません。(新機能 ; ヘルプを参照)
3>          with
3>          [
3>              T=WTF::StringImpl
3>          ]

[James Robinson]

(In reply to comment #10)
> (In reply to comment #9)
> > Can you show me a link to a failing build or post the compile errors here?
> 
> My build environment is Japanese so you might not get the error description, but the error code is C2668.
> http://msdn.microsoft.com/en-us/library/da60x087(v=VS.100).aspx
> 
> c:\cygwin\home\bashi\chrome\src\third_party\webkit\source\javascriptcore\wtf\text\StringImpl.h(173): error C2668: 'WTF::PassRefPtr<T>::PassRefPtr' : オーバーロード関数の呼び出しを解決することができません。(新機能 ; ヘルプを参照)
> 3>          with
> 3>          [
> 3>              T=WTF::StringImpl
> 3>          ]

Aha, this looks interesting (from the msdn page): "You can also get this error through template use. If, in the same class, you have a regular member function and a templated member function with the same signature, the templated one must come first. This is a limitation of the current implementation of Visual C++."

Can you try swapping these two lines:
        PassRefPtr(std::nullptr_t) : m_ptr(0) { }
        PassRefPtr(T* ptr) : m_ptr(ptr) { refIfNotNull(ptr); }

and see if that fixes the issue for you?

[James Robinson]

So the problem is we have:
foo(void*) {}
foo(nullptr) {}

and VS2010 can't figure out what we mean when we do:
foo(0);

That's unfortunate.  I don't know any way to resolve this other than replacing all the code that does constructs a PassRefPtr<> with the literal 0 by nullptr, which will kind of suck.