[PATCH] D50341: [libcxx] Fix XFAILs for aligned allocation tests on older OSX versions

Mon Aug 13 15:18:20 PDT 2018

ldionne added inline comments.

================
Comment at: libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp:18
+// XFAIL: macosx10.8
+// XFAIL: macosx10.7

----------------
mclow.lists wrote:
> These should probably be `UNSUPPORTED` as well.
I think the correct thing for those is `XFAIL`, since this test is actually testing that the macro is defined or not defined for specific platforms. Equivalently, we could have two tests -- one `aligned_allocation_macro.pass.cpp` which is `UNSUPPORTED` on `macosx10.7` ... `macosx10.12`, and one `aligned_allocation_macro.fail.cpp` which `REQUIRES` one of `macosx10.7`, ..., `macosx10.12`. Does that make sense?

================
Comment at: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align.fail.cpp:15

 // UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
 // UNSUPPORTED: clang-3.3, clang-3.4, clang-3.5, clang-3.6, clang-3.7, clang-3.8
----------------
mclow.lists wrote:
> ldionne wrote:
> > vsapsai wrote:
> > > In what cases are we supposed to run these tests? Such extensive collection of unsupported C++ standards looks suspicious and I guess that is the reason why I haven't seen test failures with older libc++ dylibs.
> > That's an excellent question. I would assume those should be enabled in C++17 and above, I'm not sure why they're disabled. @mclow.lists was the one to introduce those tests, perhaps he can shed light on why they were disabled in C++17?
> IIRC, this call - `operator new(std::size_t, std::align_val_t);` was introduced for C++17, and `[[nodiscard]]` was added for C++20.
> 
> this test is making sure that we get a warning when you call this and don't use the return value. It requires C++20 and above, hence the massive UNSUPPORTED list
Wouldn't it be sufficient to list the standard required? Why do we have `UNSUPPORTED` compilers too?

Repository:
  rCXX libc++

https://reviews.llvm.org/D50341