[Bug c++/56373] New: -Wzero-as-null-pointer-constant: does not catch issues with smart pointers

[Bug c++/56373] New: -Wzero-as-null-pointer-constant: does not catch issues with smart pointers

[akim.demaille at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56373

             Bug #: 56373
           Summary: -Wzero-as-null-pointer-constant: does not catch issues
                    with smart pointers
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: minor
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: akim.demaille@gmail.com


Hi all,

Sorry if this report is bogus.  I would expect GCC to complain about the
following uses of 0 instead of nullptr, but it does not.

$ cat foo.cc
#include <memory>
struct foo {};

int main ()
{
  std::shared_ptr<foo> a = 0;
  std::shared_ptr<foo> b(0);
  std::shared_ptr<foo> c{0};
  foo *d = 0;
}
$ g++-mp-4.8 -std=c++11 -Wall -Wzero-as-null-pointer-constant /tmp/foo.cc 
/tmp/foo.cc: In function 'int main()':
/tmp/foo.cc:9:12: warning: zero as null pointer constant
[-Wzero-as-null-pointer-constant]
   foo *d = 0;
            ^
/tmp/foo.cc:9:8: warning: unused variable 'd' [-Wunused-variable]
   foo *d = 0;
        ^
$ g++-mp-4.8  --version
g++-mp-4.8 (MacPorts gcc48 4.8-20130210_0) 4.8.0 20130210 (experimental)
Copyright (C) 2013 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

It's also a bit sad that only d is diagnosed as useless, although I do
understand that the shared_ptr has a constructor and a destructor that use
it.

Cheers!

[Bug c++/56373] -Wzero-as-null-pointer-constant: does not catch issues with smart pointers

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56373

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> 2013-02-18 12:07:49 UTC ---
The warning isn't issued when 0 converts to std::nullptr_t, only when it
converts to a pointer type.

struct shared_ptr
{
    shared_ptr(decltype(nullptr)) { }
    ~shared_ptr() { }
};

int main ()
{
  shared_ptr a = 0;
  shared_ptr b(0);
  shared_ptr c{0};
}


(In reply to comment #0)
> It's also a bit sad that only d is diagnosed as useless, although I do
> understand that the shared_ptr has a constructor and a destructor that use
> it.

It's necessary, because otherwise you get bogus warnings from ScopeGuard-style
RAII types.

[Bug c++/56373] -Wzero-as-null-pointer-constant: does not catch issues with smart pointers

[akim.demaille at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56373

--- Comment #2 from Akim Demaille <akim.demaille at gmail dot com> 2013-02-18 12:52:46 UTC ---
Thanks a lot for the detailed answer.

> The warning isn't issued when 0 converts to std::nullptr_t, only when it
> converts to a pointer type.

And shouldn't it?

>> It's also a bit sad that only d is diagnosed as useless, although I do
>> understand that the shared_ptr has a constructor and a destructor that use
>> it.

> It's necessary, because otherwise you get bogus warnings from ScopeGuard-style
> RAII types.

In which case the constructor and destructor would be meaningful,
which is not the case here.  But again, thanks for explaining!

[Bug c++/56373] -Wzero-as-null-pointer-constant: does not catch issues with smart pointers

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56373

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> 2013-02-18 13:20:41 UTC ---
(In reply to comment #2)
> > It's necessary, because otherwise you get bogus warnings from ScopeGuard-style
> > RAII types.
> 
> In which case the constructor and destructor would be meaningful,
> which is not the case here.

~shared_ptr() has non-trivial side-effects, the compiler isn't smart enough to
determine they won't fire when its empty, so it's always meaningful.

If you're smart enough to know the object isn't used then don't create it :)

[Bug c++/56373] -Wzero-as-null-pointer-constant: does not catch issues with smart pointers

[akim.demaille at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56373

--- Comment #4 from Akim Demaille <akim.demaille at gmail dot com> 2013-02-18 13:23:08 UTC ---
> If you're smart enough to know the object isn't used then don't create it :)

:) :) :)

> ~shared_ptr() has non-trivial side-effects, the compiler isn't smart enough to
> determine they won't fire when its empty, so it's always meaningful.

I had in mind providing the library authors with an attribute that would
help them influence this diagnostic.

[Bug c++/56373] -Wzero-as-null-pointer-constant: does not catch issues with smart pointers

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56373

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-02-18
         AssignedTo|unassigned at gcc dot       |paolo.carlini at oracle dot
                   |gnu.org                     |com
     Ever Confirmed|0                           |1

--- Comment #5 from Paolo Carlini <paolo.carlini at oracle dot com> 2013-02-18 16:05:40 UTC ---
So confirming the first issue. I have a simple patch in testing for it.

About the second issue, it was definitely discussed somewhere else too: there
is always a tension between consistently treating in a similar way elementary
types and classes and avoiding warning for common patterns like RAII.

[Bug c++/56373] -Wzero-as-null-pointer-constant: does not catch issues with smart pointers

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56373

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

[Bug c++/56373] -Wzero-as-null-pointer-constant: does not catch issues with smart pointers

[paolo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56373

--- Comment #6 from paolo at gcc dot gnu.org <paolo at gcc dot gnu.org> 2013-02-20 09:02:41 UTC ---
Author: paolo
Date: Wed Feb 20 09:02:35 2013
New Revision: 196165

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=196165
Log:
/cp
2013-02-20  Paolo Carlini  <paolo.carlini@oracle.com>

    PR c++/56373
    * tree.c (maybe_warn_zero_as_null_pointer_constant): Add.
    * cvt.c (ocp_convert): Use the latter.
    (cp_convert_to_pointer): Likewise.
    * decl.c (check_default_argument): Likewise.
    * typeck.c (cp_build_binary_op): Likewise.
    * cp-tree.h (maybe_warn_zero_as_null_pointer_constant): Declare.

/testsuite
2013-02-20  Paolo Carlini  <paolo.carlini@oracle.com>

    PR c++/56373
    * g++.dg/cpp0x/Wzero-as-null-pointer-constant-2.C: New.

Added:
    trunk/gcc/testsuite/g++.dg/cpp0x/Wzero-as-null-pointer-constant-2.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/cp-tree.h
    trunk/gcc/cp/cvt.c
    trunk/gcc/cp/decl.c
    trunk/gcc/cp/tree.c
    trunk/gcc/cp/typeck.c
    trunk/gcc/testsuite/ChangeLog

[Bug c++/56373] -Wzero-as-null-pointer-constant: does not catch issues with smart pointers

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56373

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.8.0

--- Comment #7 from Paolo Carlini <paolo.carlini at oracle dot com> 2013-02-20 09:03:47 UTC ---
Done.