Quuxplusone added inline comments.
================
Comment at: src/experimental/memory_resource.cpp:29
+static bool is_aligned_to(void *ptr, size_t align)
+{
+ void *p2 = ptr;
----------------
EricWF wrote:
> Wait, can't this be written `reinterpret_cast<uintptr_t>(ptr) % align == 0`?
Yes on sane platforms, but that's technically implementation-defined behavior:
the low bits of the uintptr_t may not correspond to the "alignment" of the
original pointer (for example, on some hypothetical tagged-pointer
architecture?). I don't know if libc++ cares about those platforms, but it
seems like `std::align` was added specifically to solve this problem in a
portable way, so I thought it would be best to use it.
If you reply and say "yeah but please change it anyway," I'll change it anyway.
:)
================
Comment at: src/experimental/memory_resource.cpp:48
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+ if (__size != 0 && !is_aligned_to(result, __align)) {
+ _VSTD::__libcpp_deallocate(result, __align);
----------------
EricWF wrote:
> Also, I'm not sure about the `size != 0` check. The returned pointer may be
> non-null and incorrectly aligned.
This was covered in my commit message/summary: "If n == 0, return an
unspecified result."
However, I am chagrined to state that I have no idea where I got that idea! I
can't find support for that anywhere in the current Standard (although I'm not
sure if I missed it).
We must choose here between "allocating zero bytes sometimes returns an
underaligned pointer" and "allocating zero bytes sometimes throws bad_alloc."
Neither one seems like good QoI. But as I no longer see why I thought
"underaligned pointer" was permitted, I will change it to "sometimes throws
bad_alloc" and re-upload.
Repository:
rCXX libc++
https://reviews.llvm.org/D47344
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
