[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

Louis Dionne via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 12 11:50:08 PST 2021


ldionne requested changes to this revision.
ldionne added a comment.

Regarding the back-deployment target for Apple, I believe the first OSes where `libc++.dylib` had sized deallocation are `macOS 10.12`, `iOS 10.0`, `watchOS 3.0`, and `tvOS 10.0`. We should be able to enable reliance on sized deallocation functions automatically whenever the deployment target is greater than or equal to these versions.

The comments I left in the test file apply to both test files.



================
Comment at: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp:12-13
 // UNSUPPORTED: sanitizer-new-delete, c++03, c++11
+// XFAIL: clang-12, clang-13
+// XFAIL: apple-clang-13
 
----------------
The comment you removed is still relevant and should be kept for these compilers, since it applies to them.


================
Comment at: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp:15-16
 
-// NOTE: Clang does not enable sized-deallocation in C++14 and beyond by
-// default. It is only enabled when -fsized-deallocation is given.
-// XFAIL: clang, apple-clang
+// REQUIRES: -fsized-deallocation
+// ADDITIONAL_COMPILE_FLAGS: -fsized-deallocation
+
----------------
This shouldn't be necessary anymore if it is enabled by default in Clang trunk, no? And in fact we definitely don't want to add these flags here, since we're trying to test the default Clang behavior. I think that's why CI is failing with a bunch of XPASSes.


================
Comment at: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp:17
+// ADDITIONAL_COMPILE_FLAGS: -fsized-deallocation
+
+#if !defined(__cpp_sized_deallocation)
----------------
We will also need to add the following annotation to express that the test is expected to fail when run on targets where sized deallocation wasn't available yet:

```
// Sized deallocation was added in macOS 10.12 and aligned OSes.
// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11}}
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921



More information about the cfe-commits mailing list