Quuxplusone marked 5 inline comments as done. Quuxplusone added inline comments.
================ 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); ---------------- Quuxplusone wrote: > Quuxplusone wrote: > > 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. > Hm! This and your other comment play into each other unfortunately well. When > `size == 0`, the pointer we get back from `__libcpp_allocate` actually points > to zero bytes, which means that when I call `std::align` on it, I invoke > undefined behavior. > > My new theory is "I don't care about that undefined behavior." I'd still > rather take undefined-behavior-in-a-rare-case over > implementation-defined-behavior-in-all-cases, and I'm too lazy to implement > implementation-defined-behavior-only-in-a-rare-case unless you tell me to. :P Now fixed. (The fix is always in the last place you look!) 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