[Bug libstdc++/100903] New: Bogus "zero as null pointer constant" warning

[Bug libstdc++/100903] New: Bogus "zero as null pointer constant" warning

[terra at gnome dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100903

            Bug ID: 100903
           Summary: Bogus "zero as null pointer constant" warning
           Product: gcc
           Version: 11.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: terra at gnome dot org
  Target Milestone: ---

Created attachment 50926
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50926&action=edit
Preprocesses source code

g++ emits a bogus warning when a std::partial_ordering value is compared
to a literal 0.  See also bug 95242.

The standard is crazy-strict here.  Anything other than a literal 0 is
undefined behaviour.  Allowed: 0, 0LL, 0'0'0'0, 0x0.  No good: +0, nullptr,
(void*)0, 0+0, '\0', (0,nullptr).  gcc incorrectly allows nullptr and
(0,nullptr), but rejects the others.

The library tries to do that with a construct that mainly allows null pointer
constants.  That leads to the warning when you actually supply the 0 you are
supposed to.

Suggestion: just use an int argument.  That's wrong in different ways (and
might cause warnings with 0LL which is allowed), but it matches 0 better. 
Doing this right is likely to require compiler support.



# cat uuu.C
#include <compare>

int
foo(std::partial_ordering po)
{
  return po < 0;
}

# g++ -Wall -std=gnu++20 -Wzero-as-null-pointer-constant -c uuu.C
uuu.C: In function ‘int foo(std::partial_ordering)’:
uuu.C:6:15: warning: zero as null pointer constant
[-Wzero-as-null-pointer-constant]
    6 |   return po < 0;
      |               ^


# g++ -v
Using built-in specs.
COLLECT_GCC=/usr/local/products/gcc/11.1.0/bin/g++
COLLECT_LTO_WRAPPER=/usr/local/products/gcc/11.1.0/lib/gcc/x86_64-suse-linux/11.1.0/lto-wrapper
Target: x86_64-suse-linux
Configured with: ../../gcc-11.1.0/configure --enable-languages=c,c++,fortran
--enable-targets=x86_64-suse-linux,i686-suse-linux
--prefix=/usr/local/products/gcc/11.1.0 --with-gnu-as
--with-as=/usr/local/products/gcc/binutils-2.36.1/bin/as --with-gnu-ld
--with-ld=/usr/local/products/gcc/binutils-2.36.1/bin/ld --enable-link-mutex
--enable-gnu-indirect-functions --enable-linux-futex --enable-threads=posix
--enable-shared --enable-__cxa_atexit --enable-libstdcxx-allocator=new
x86_64-suse-linux
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 11.1.0 (GCC)

[Bug libstdc++/100903] Bogus "zero as null pointer constant" warning

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100903

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
http://eel.is/c++draft/cmp.categories.pre#example-1
lists nullptr_t as an example of a type that satisfies the requirements.
GCC does not reject 0, just can emit a non-default warning about it if you
enable that warning.

[Bug libstdc++/100903] Bogus "zero as null pointer constant" warning

[terra at gnome dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100903

--- Comment #2 from M Welinder <terra at gnome dot org> ---
IMHO, nullptr_t would not be an improvement here.  We would still have the
combination of:

(1) Correct usage causing a warning
(2) The warning hinting at using the incorrect nullptr instead.
(3) po<nullptr compiling with no error or even warning.

I think the current magic-pointer code could be improved with something like

    template<class T,std::enable_if_t<std::is_same_v<T,std::nullptr_t>>>
    bool operator< (..., T) = delete;

This would at least take care of (3).  (Template magic applied to resolve the
ambiguity of what to do with literal 0.  There might be a better way.)

[Bug libstdc++/100903] Bogus "zero as null pointer constant" warning

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100903

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
This is a dup of PR 99778

[Bug libstdc++/100903] Bogus "zero as null pointer constant" warning

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100903

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |headch at gmail dot com

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
*** Bug 99778 has been marked as a duplicate of this bug. ***

[Bug libstdc++/100903] Bogus "zero as null pointer constant" warning

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100903

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to M Welinder from comment #2)
> I think the current magic-pointer code could be improved with something like
> 
>     template<class T,std::enable_if_t<std::is_same_v<T,std::nullptr_t>>>
>     bool operator< (..., T) = delete;

Or another constructor for the __cmp_cat::__unspec type. I did try that at one
point.

The current code was the least bad solution of several that I tried. I didn't
notice the -Wzero-as-null-pointer-constant warning because it's not enabled by
default (because it's about coding style, not correctness).

[Bug libstdc++/100903] Bogus "zero as null pointer constant" warning

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100903

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-06-04

[Bug libstdc++/100903] Bogus "zero as null pointer constant" warning

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100903

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu.org,
                   |                            |mpolacek at gcc dot gnu.org,
                   |                            |ppalka at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, instead of adding a new compiler extension, couldn't we just add a hack for
this warning and temporarily disable the -Wzero-as-null-pointer-constant
warning
while doing convert_like_internal to std::__cmp_cat::__unspec convs->type, or
while build_over_call to the std::__cmp_cat::__unspec ctor?
Or add some attribute to that ctor which would cause the warning to be
temporarily disabled while handling its argument.

[Bug libstdc++/100903] Bogus "zero as null pointer constant" warning

[marc.mutz at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100903

Marc Mutz <marc.mutz at hotmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marc.mutz at hotmail dot com

--- Comment #7 from Marc Mutz <marc.mutz at hotmail dot com> ---
In Qt, we still only require C++17, but we want to write our relational
operators as-if we already had C++20 and use macros to paper over the
difference. This requires us to write our own C++17 versions of std::_*ordering
(called Qt:*_ordering). Whatever you do to fix the issue, please don't only fix
it for libstdc++, but also for user code.

[Bug libstdc++/100903] Bogus "zero as null pointer constant" warning

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100903

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
For Qt you have the option of just using int as the type that the comparison 
category classes compare to.

[Bug libstdc++/100903] Bogus "zero as null pointer constant" warning

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100903

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to M Welinder from comment #0)
> The standard is crazy-strict here.  Anything other than a literal 0 is
> undefined behaviour.  Allowed: 0, 0LL, 0'0'0'0, 0x0.

I don't think 0LL is allowed. A literal 0 has type int, and 0LL doesn't. I'm
unsure about 0'0 and 0x0.

>  No good: +0, nullptr,
> (void*)0, 0+0, '\0', (0,nullptr).  gcc incorrectly allows nullptr and
> (0,nullptr), but rejects the others.

gcc is not incorrect, the code that uses nullptr or (0, nullptr) is incorrect.
No diagnostic is required for violations of that rule.

> The library tries to do that with a construct that mainly allows null
> pointer constants.  That leads to the warning when you actually supply the 0
> you are supposed to.
> 
> Suggestion: just use an int argument.  That's wrong in different ways (and
> might cause warnings with 0LL which is allowed), but it matches 0 better.

Using int would allow 1 and 100 though. That seems much worse than allowing
other spellings of zero.

However, since I added the <compare> header gcc gained support for consteval
functions, which would allow us to reject non-zero values.

Something like this should work:

    struct __unspec
    {
      template<same_as<int> _Tp>
        consteval __unspec(_Tp __z) noexcept
        {
          if (__z != 0) throw; // comparison category types only compare to 0
        }
    };

This results in errors like:

cmp.cc:6:13: error: no match for 'operator<' (operand types are
'std::partial_ordering' and 'long int')
    6 |   return po < 0L;
      |          ~~ ^ ~~
      |          |    |
      |          |    long int
      |          std::partial_ordering

and:

/home/jwakely/gcc/14/include/c++/14.0.0/compare:59:25: error: expression
'<throw-expression>' is not a constant expression
   59 |           if (__z != 0) throw; // comparison category types only
compare to 0
      |                         ^~~~~

[Bug libstdc++/100903] Bogus "zero as null pointer constant" warning

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100903

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It could also help to overload the comparison operators with constrained
deleted overloads, like so:

    template<typename _Tp>
      concept __not_literal_zero = !convertible_to<_Tp, __unspec>;

    friend constexpr bool
    operator==(partial_ordering, __cmp_cat::__not_literal_zero auto) = delete;

    friend constexpr void
    operator<=>(partial_ordering, __cmp_cat::__not_literal_zero auto) = delete;


With that the "no match for 'operator<'" error  (and its eight accompanying
notes) become:


cmp.cc:7:15: error: use of deleted function 'constexpr void
std::operator<=>(partial_ordering, auto:3) [with auto:3 = long int;
partial_ordering = partial_ordering]'
    7 |   return po > 0L;
      |               ^~
/home/jwakely/gcc/14/include/c++/14.0.0/compare:160:5: note: declared here
  160 |     operator<=>(partial_ordering, __cmp_cat::__not_literal_zero auto) =
delete;
      |     ^~~~~~~~

[Bug libstdc++/100903] Bogus "zero as null pointer constant" warning

[terra at gnome dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100903

--- Comment #11 from M Welinder <terra at gnome dot org> ---
>/home/jwakely/gcc/14/include/c++/14.0.0/compare:160:5: note: declared here
>  160 |     operator<=>(partial_ordering, __cmp_cat::__not_literal_zero auto) = delete;
>      |     ^~~~~~~~

I don't think we can hope for much more than this.  That's almost readable!


You might want to look at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95242#c2
and following comments.

[Bug libstdc++/100903] Bogus "zero as null pointer constant" warning

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100903

--- Comment #12 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Drat ... so that's why I didn't do it already.

[Bug libstdc++/100903] Bogus "zero as null pointer constant" warning

[headch at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100903

--- Comment #13 from Christopher Head <headch at gmail dot com> ---
Could the error message be made more readable by trying to call an
undefined-but-declared non-consteval function inside the consteval context in
the error case, rather than throwing? The name of the function could then be
the error message? For example:


struct __unspec
{
  static void __comparisonCategoryComparedToNonZeroValue();

  template<same_as<int> _Tp>
  consteval __unspec(_Tp __z) noexcept
  {
    if (__z != 0) __comparisonCategoryComparedToNonZeroValue();
  }
};

You then get the error “call to non-‘constexpr’ function ‘static void
__unspec::__comparisonCategoryComparedToNonZeroValue()’”

[Bug libstdc++/100903] Bogus "zero as null pointer constant" warning

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100903

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Yes, that part's easy (and that's what we do in std::format for errors during
format string parsing). But accepting (a <=> b) < (1-1) and other zero-valued
constant expressions can't be solved by improving the diagnostics for
ill-formed cases.

(In reply to Jakub Jelinek from comment #6)
> So, instead of adding a new compiler extension, couldn't we just add a hack
> for this warning and temporarily disable the -Wzero-as-null-pointer-constant
> warning
> while doing convert_like_internal to std::__cmp_cat::__unspec convs->type, or
> while build_over_call to the std::__cmp_cat::__unspec ctor?
> Or add some attribute to that ctor which would cause the warning to be
> temporarily disabled while handling its argument.

This would be my preference.