[libcxx-commits] [PATCH] D137127: [libc++abi] Use std::nullptr_t instead of declaring it manually

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF added a comment.

In D137127#3900823 <https://reviews.llvm.org/D137127#3900823>, @rprichard wrote:

> In D137127#3900589 <https://reviews.llvm.org/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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137127