[libcxx-commits] [PATCH] D146372: [libc++]Enforce `-Wzero-as-null-pointer-constant`

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF requested changes to this revision.
EricWF added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__compare/ordering.h:46
+  consteval
+  _CmpUnspecifiedParam(int __literal_zero) noexcept {
+    if (__literal_zero != 0) {
----------------
Richard Smith and I have talked about creating a special type in Clang to use here.
I think we should investigate this approach more thoroughly before going down the path suggested here.

As is, your change makes the number of values and types accepted as an argument to `_CmpUnspecifiedParam` MUCH larger.

Before, it would only accept the _LITERAL_ zero, now it accepts anything that evaluates to zero, including `(0 + 42 - 42)`, `foo()`, etc.

So this makes the library much less conforming than it was. This cannot proceed as is.

Though I grant it's very frustrating that the warning is triggered here. But we'll need Clang's help to resolve that.


================
Comment at: libcxx/test/libcxx/language.support/cmp/cmp.categories.pre/zero_type.verify.cpp:22
   void(v op nullptr);                                                          \
-  void(nullptr op v);                                                          \
-  void(v op(1 - 1));                                                           \
-  void((1 - 1) op v)
+  void(nullptr op v)
 
----------------
This change seems wrong. It looks like we're enforcing that an error actually occurs when zero is used.
But those test cases get removed?

Maybe we should disable the warning in this test.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146372/new/

https://reviews.llvm.org/D146372