vsapsai added inline comments.

================
Comment at: libcxx/include/__config:993
+    !defined(_LIBCPP_BUILDING_LIBRARY) && \
+    !defined(__cpp_aligned_new)
+#  define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
----------------
I think I'd rather keep `__cpp_aligned_new >= 201606` check. I don't know about 
cases where the smaller value is possible, Clang always had 201606 as far as I 
(and SVN) know. But it seems to be safer and more correct.

Don't have objective arguments for keeping the check, so if you think it's not 
worth it, I trust your judgement.


================
Comment at: libcxx/include/new:111-116
 #if !__has_builtin(__builtin_operator_new) || \
    __has_builtin(__builtin_operator_new) < 201802L || \
    defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) || \
    !defined(__cpp_aligned_new) || __cpp_aligned_new < 201606
 #define _LIBCPP_HAS_NO_BUILTIN_ALIGNED_OPERATOR_NEW_DELETE
 #endif
----------------
Maybe move this to `__config` too? This way we'll have 
`__cpp_aligned_new`-related macros together.


================
Comment at: libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp:11
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+// XFAIL: with_system_cxx_lib=macosx10.12
+// XFAIL: with_system_cxx_lib=macosx10.11
----------------
Initially `with_system_cxx_lib` made me suspicious because macro 
`_LIBCPP_HAS_NO_ALIGNED_ALLOCATION` doesn't need specific dylib. And `XFAIL: 
availability=macosx10.12` seems to be working.

[Documentation](https://github.com/llvm-mirror/libcxx/blob/master/docs/DesignDocs/AvailabilityMarkup.rst#testing)
 describes which feature should be used in different cases but in this case I 
cannot definitely say if test uses unavailable feature. I think it is 
acceptable to stick with `with_system_cxx_lib` but I decided to brought to your 
attention the alternative.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50344



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to