[libcxx-commits] [PATCH] D150408: [libc++] Complete refactor of tests for operator new

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size.nodiscard.verify.cpp:9
 
-// <new>
-
-// void* operator new[](std::size_t);
+// [[nodiscard]] void* operator new[](std::size_t);
 
----------------
We have this as an extension now, so we might as well enable this test for libc++ in all language versions.


================
Comment at: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size.pass.cpp:26
+    ++new_handler_called;
+    std::set_new_handler(0);
+}
----------------



================
Comment at: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size.replace.indirect.pass.cpp:29
+TEST_WORKAROUND_BUG_109234844_WEAK
+void* operator new(std::size_t s) TEST_THROW_SPEC(std::bad_alloc) {
     ++new_called;
----------------
Are you allowed to override only operator new //or// operator delete? It yes, we might want to add a test for that.


================
Comment at: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.nodiscard.verify.cpp:23
 void f() {
     ::operator new[](4, std::align_val_t{4});  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
 }
----------------
Is there a reason the `[[nodiscard]]` tests aren't merged?


================
Comment at: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.replace.indirect.pass.cpp:36
+TEST_WORKAROUND_BUG_109234844_WEAK
+void* operator new(std::size_t s, std::align_val_t a) TEST_THROW_SPEC(std::bad_alloc) {
+    assert(s <= sizeof(DummyData));
----------------
Or is this required?


================
Comment at: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.replace.pass.cpp:58
+    }
+
+    // Test with a type that is right on the verge of being overaligned
----------------
Do we ever try calling the `align_val_t` overload explicitly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150408