[libc++abi] Use std::nullptr_t instead of declaring it manually

[rprichard]

Summary

Sometimes libc++'s stddef.h wrapper gets included, which defines
::nullptr_t. This test is compiled with -Wshadow -Werror, so shadowing
::nullptr_t with a nullptr_t in main is an error. Include cstddef,
which is guaranteed to define std::nullptr_t in C++11 and forward.

[ldionne]

LGTM with drive-by fix.

libcxxabi/test/catch_reference_nullptr.pass.cpp

9

While we're at it.

[rprichard]

Remove extraneous comma.

[EricWF]

I don't think this helps, but I don't think it hurts. The correctness of the tests requires std::nullptr_t to be decltype(nullptr), so /shrug

[rprichard]

In D137127#3900589, @EricWF wrote:

I don't think this helps, but I don't think it hurts. The correctness of the tests requires std::nullptr_t to be decltype(nullptr), so /shrug

The question is whether the <cstdlib> header is allowed to define a ::nullptr_t. If it does define it (it does with the Android NDK), then the test doesn't compile because -Wshadow complains about the local nullptr_t. (I suspect the issue is that bionic's stdlib.h includes malloc.h, which includes stddef.h without first declaring any __need_* macros. When libc++'s stddef.h is included, it defines ::nullptr_t.) It could be an issue with the bionic headers, but that's also probably hard to clean up without breaking stuff?

[EricWF]

In D137127#3900823, @rprichard wrote:

In D137127#3900589, @EricWF wrote:

I don't think this helps, but I don't think it hurts. The correctness of the tests requires std::nullptr_t to be decltype(nullptr), so /shrug

The question is whether the <cstdlib> header is allowed to define a ::nullptr_t. If it does define it (it does with the Android NDK), then the test doesn't compile because -Wshadow complains about the local nullptr_t. (I suspect the issue is that bionic's stdlib.h includes malloc.h, which includes stddef.h without first declaring any __need_* macros. When libc++'s stddef.h is included, it defines ::nullptr_t.) It could be an issue with the bionic headers, but that's also probably hard to clean up without breaking stuff?

I think this change is fine to proceed. Maybe for the super paranoid of us (me), we could static_assert(is_same<std::nullptr_t, decltype(nullptr)>).

That said, you're also free to ignore everything I just said. This test would probably fail if the types weren't the same, and it'll be a very weird day if the bug I'm trying to defend against actually comes to pass.

[rprichard]

Add static_assert

[rprichard]

Avoid std::is_same_v, which was added in C++17, in favor of std::is_same::value.

libcxxabi/test/catch_reference_nullptr.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: c++03,
 // UNSUPPORTED: no-exceptions
 
 #include <cassert>
+#include <cstddef>
 #include <cstdlib>
 
 struct A {};
 
 template<typename T, bool CanCatchNullptr>
 static void catch_nullptr_test() {
   try {
     throw nullptr;
   } catch (T &p) {
     assert(CanCatchNullptr && !static_cast<bool>(p));
   } catch (...) {
     assert(!CanCatchNullptr);
   }
 }
 
 int main(int, char**)
 {
-  using nullptr_t = decltype(nullptr);
-
   // A reference to nullptr_t can catch nullptr.
-  catch_nullptr_test<nullptr_t, true>();
-  catch_nullptr_test<const nullptr_t, true>();
-  catch_nullptr_test<volatile nullptr_t, true>();
-  catch_nullptr_test<const volatile nullptr_t, true>();
+  catch_nullptr_test<std::nullptr_t, true>();
+  catch_nullptr_test<const std::nullptr_t, true>();
+  catch_nullptr_test<volatile std::nullptr_t, true>();
+  catch_nullptr_test<const volatile std::nullptr_t, true>();
 
   // No other reference type can.
 #if 0
   // FIXME: These tests fail, because the ABI provides no way for us to
   // distinguish this from catching by value.
   catch_nullptr_test<void *, false>();
   catch_nullptr_test<void * const, false>();
   catch_nullptr_test<int *, false>();
   catch_nullptr_test<A *, false>();
   catch_nullptr_test<int A::*, false>();
   catch_nullptr_test<int (A::*)(), false>();
 #endif
 
   return 0;
 }