Wzero-as-null-pointer-constant warns when using operator<=>

[davidstone]

Bugzilla Link	44325
Version	unspecified
OS	Linux
CC	@adrianimboden,@dwblaikie,@0x8000-0000,@dvhwgumby,@mclow,@mwinterb,@mizvekov,@zygoloid,@Weverything
Fixed by commit(s)	1f06f41, 678c259

Extended Description

#include <compare>

auto b = 1 <=> 2 < 0;

Outputs:

<source>:3:20: warning: zero as null pointer constant [-Wzero-as-null-pointer-constant]

auto b = 1 <=> 2 < 0;

                   ^

                   nullptr

1 warning generated.

Compiler returned: 0

See it live: https://godbolt.org/z/9a8cnD

Technically, the warning is accurate. User code has a literal 0 that is being passed to a function accepting a nullptr. However, this is an implementation detail of the standard library and the intended interface for use (users should not use nullptr there), so we should not warn users for doing so (and definitely should not direct them to change their code).

[zygoloid]

Retargeting as a libc++ bug.

Using a pointer-to-member type here seems like a bad choice. This would probably be better:

// Empty struct so not passed at runtime.
struct __zero_type {
  // Only accept a constant integer 0 argument.
  constexpr __zero_type(int n)
    __attribute__((enable_if(n == 0, "comparison only supported with literal 0")))
  {}

  // Reject any other type; reject int lvalues.
  template<typename T> __zero_type(T&&) = delete;
  template<typename T> __zero_type(const volatile int&) = delete;
};

// Do not accept class types that convert to int
// (that would require two user-defined conversions).
bool operator<(std::strong_ordering s, __zero_type) 

This is not quite perfect; we'd accept

  1 <=> 2 < 3 - 3

If that's a concern, we can certainly add an extension to Clang to do this perfectly. (For example, we could add a built-in __zero_type, or an attribute for an int parameter to restrict it to being a literal zero.)

[adrianimboden]

This one is similar, but has nothing to do with the libc++ implementation? (https://godbolt.org/z/b5fCvj):

  #include <compare>
  
  class MyStruct {
  public:
      auto operator<=>(MyStruct const& other) const = default;
  };
  
  
  bool foo(MyStruct bar){
      return bar <= bar;
  }

[adrianimboden]

This one is similar, but has nothing to do with the libc++ implementation?
(https://godbolt.org/z/b5fCvj):

  #include <compare>
  
  class MyStruct {
  public:
      auto operator<=>(MyStruct const& other) const = default;
  };
  
  
  bool foo(MyStruct bar){
      return bar <= bar;
  }

Ah I see, the generated code which uses std::strong_ordering from stdlib just is not visible in the error message.

[0x8000-0000]

The problem might not be in libc++, because

#include <chrono>

using DeviceTimePoint = std::chrono::microseconds;

bool busyLoop(std::chrono::microseconds timeOut);

DeviceTimePoint getTime();

bool isBusy();

bool busyLoop(std::chrono::microseconds timeOut)
{
    bool status = true;
    const auto stopTime = getTime() + timeOut;

    while (isBusy())
    {
        if (getTime() >= stopTime)
        {
            status = false;
            break;
        }
    }
    return status;
}

Produces this:

$ /opt/clang11/bin/clang++ --gcc-toolchain=/opt/gcc10 -std=c++20 -c -Werror -Wzero-as-null-pointer-constant compare_durations.cpp
compare_durations.cpp:18:23: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant]
        if (getTime() >= stopTime)
                      ^~
                      nullptr
/opt/gcc10/lib/gcc/x86_64-pc-linux-gnu/10.1.0/../../../../include/c++/10.1.0/chrono:677:7: note: while rewriting comparison as call to 'operator<=>' declared here
      operator<=>(const duration<_Rep1, _Period1>& __lhs,
      ^
1 error generated.

[zygoloid]

compare_durations.cpp:18:23: error: zero as null pointer constant
[-Werror,-Wzero-as-null-pointer-constant]
        if (getTime() >= stopTime)
                      ^~
                      nullptr
/opt/gcc10/lib/gcc/x86_64-pc-linux-gnu/10.1.0/../../../../include/c++/10.1.0/
chrono:677:7: note: while rewriting comparison as call to 'operator<=>'
declared here
      operator<=>(const duration<_Rep1, _Period1>& __lhs,
      ^

It looks like libstdc++ has the same problem as libc++ in this regard. It would certainly seem reasonable to suppress these warnings, since we didn't manage to get the standard libraries fixed in time. :(

[zygoloid]

*** Bug llvm/llvm-bugzilla-archive#46803 has been marked as a duplicate of this bug. ***

[zygoloid]

CC rtrieu, who was looking at this in the context of the duplicate bug.

We probably track some state in Sema to disable this warning while building a <=> operator. Alternatively, we could hard-code knowledge of the exact type names used here by libc++ and libstdc++, but I think similar patterns are likely to emerge for user-authored comparison category types, so the more general approach seems preferable.

[mwinterb]

CC rtrieu, who was looking at this in the context of the duplicate bug.

We probably track some state in Sema to disable this warning while building
a <=> operator. Alternatively, we could hard-code knowledge of the exact
type names used here by libc++ and libstdc++, but I think similar patterns
are likely to emerge for user-authored comparison category types, so the
more general approach seems preferable.

As more data: for clang-cl, Microsoft's implementation just uses a type alias for decltype(nullptr) (distinct from std::nullptr_t) to handle this:
https://github.com/microsoft/STL/blob/1469bf36dcc01a512d3297a06d25c7acbb641bc3/stl/inc/compare#L30

[llvmbot]

Here is an example that might also relate to this:

std::vector<int> v{1,2,1,2,1,2};
for (auto it = v.cbegin(); (it = find_if(it, v.cend(), somePredicate)) < v.cend(); ++it) {
    std::cout << *it << std::endl;
}

See: https://godbolt.org/z/15bqMj

See: https://stackoverflow.com/questions/64800254/warning-zero-as-null-pointer-constant-while-comparing-iterators/64800675#64800675

[zygoloid]

*** Bug llvm/llvm-bugzilla-archive#48281 has been marked as a duplicate of this bug. ***

[llvmbot]

Btw., in Visual Studio 16.8, this also happens with clang-cl.

In my opinion, this is a really a C++ language defect, as std::strong_ordering etc. must be comparable with literal 0, but there is no way to cleanly implement that, because literal zero doesn't have its own type. So standard libraries are pretty much forced to implement this by interpreting the zero as a pointer.

Given that, I think it would be justifiable just to suppress the warning for the special cases where the C++ standard requires comparison a literal zero. We know the implementations pretty much have no choice but to do this, and we know that the code is correct.

I would suggest simple suppressing the warning for the comparison operators of std::strong_ordering, std::weak_ordering, and std::partial_ordering. And if users need to define their own categories, they can alwys wrap them in pragmas to disable the warning as well.

[llvmbot]

Should this issue really be classified as an enhancement? It makes -Wzero-as-null-pointer-constant completely unusable. You can't even compare two std::strings without triggering the warning. What good is a warning that flags the
following code:

#include <string>

auto test(const std::string &a, const std::string &b)
{
	return a < b;
}

Compiled with -std=c++20 -Wzero-as-null-pointer-constant under Ubuntu 20.04 on amd64 WSL with:

clang-11 version 1:11.0.1~++20201121072613+973b95e0a84-1exp120201121063226.132
libstdc++-10-dev:amd64 version 10.2.0-5ubuntu1~20.04

[zygoloid]

*** Bug llvm/llvm-bugzilla-archive#49008 has been marked as a duplicate of this bug. ***

[zygoloid]

Fixed for trunk and Clang 12 release branch.

[davidstone]

My test case still triggers the warning with libc++ and libstdc++ as of 3e1317f. Has the fix landed in trunk?

[zygoloid]

My test case still triggers the warning with libc++ and libstdc++ as of
3e1317f. Has the fix landed in trunk?

Yes, but the fix doesn't fix that case. (Ironic, given that's comment#0 of this bug.)

We no longer warn when a comparison against a 0 literal is generated by rewriting some other comparison operator in terms of <=> (eg, comment#2, comment#4, comment#9, comment#12), but we still warn for a direct comparison of 0 against a comparison category (or anything else) where the comparison operator takes a pointer type.

It's not clear to me what we can do to suppress the remaining warnings -- I don't really want to hardcode the three comparison category types and leave user code to fend for itself, and asking for source-level annotations would be non-portable (and we'd still need to hardcode the standard comparison categories, because libstdc++ and the MS STL won't have our annotations).

Hm, I suppose we could decide that a type T is a comparison category type if ADL with T as an associated class finds an 'operator<=>' that returns T, and suppress the warning if so.

[zygoloid]

mentioned in issue llvm/llvm-bugzilla-archive#46803

[zygoloid]

mentioned in issue llvm/llvm-bugzilla-archive#48281

[zygoloid]

mentioned in issue llvm/llvm-bugzilla-archive#49008

[mwinterb]

mentioned in issue llvm/llvm-bugzilla-archive#50877

[DeadlyRedCube]

In Clang 14.0.1 I'm getting this without an explicit comparison against 0, using an auto-generated operator<=>:

#include <chrono>

class Wrapper
{
public:
    std::strong_ordering operator<=>(const Wrapper &other) const noexcept = default;

private:
    std::chrono::system_clock::time_point t;
};



bool Foo()
{
    Wrapper a, b;
    return a < b;
}

This code doesn't have any explicit 0 checks in it, it's all in the autogenerated code (here's a compiler explorer link, if that helps:
https://godbolt.org/z/jWxhdMz8a.

I ended up working around it by writing a custom <=> (that just returns (t <=> other.t)) and then a default operator==, which works but definitely isn't ideal.

[frederick-vs-ja]

It's unfortunate that we need to either accept 1 - 1 or treat 0 as a null pointer/member pointer constant. Perhaps we should ask the Clang frontend to add an internal type which can only be initialized from literal 0.

[mordante]

I tested this with clang-tidy-18 recently and it didn't issue a diagnostic about this. Can you test with clang-18?

[frederick-vs-ja]

The warning is still triggered with options -std=c++20 -stdlib=libc++ -Wzero-as-null-pointer-constant (Godbolt link).

[cor3ntin]

@mordante @ldionne As unpalatable as i think it is, adding an attribute in the front-end might be the most robust solution, it it helps and can be done, ABI wise along the line of void f(int [[clang::literal_zero]] i);.
Let me know what you think, i might have time for clang 19

[ldionne]

@cor3ntin I'm trying something out in #79465 . Please let me know what you think about it. We could indeed use [[clang::literal_zero]] to make libc++ a bit stricter if we go down the route of my PR.