[libcxx-commits] [PATCH] D107755: [libcxx] [test] Generalize defines for skipping allocation checks

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new_align_val_t_nothrow_replace.pass.cpp:65-68
+    (void)p;
+    ASSERT_WITH_LIBRARY_INTERNAL_ALLOCATIONS(p == Buff);
     assert(static_cast<std::size_t>(a) == OverAligned);
+    ASSERT_WITH_LIBRARY_INTERNAL_ALLOCATIONS(new_called);
----------------
Quuxplusone wrote:
> You shouldn't need to alter these lines.
One has to; as the overridden operator new above wasn’t called, Buff is still null, and new_called is zero.


================
Comment at: libcxx/test/support/test_macros.h:417
 #else
-#define TEST_NOT_WIN32_DLL(...) __VA_ARGS__
-#define TEST_ONLY_WIN32_DLL(...) ((void)0)
+#define ASSERT_WITH_OPERATOR_NEW_FALLBACKS(...) assert(__VA_ARGS__)
 #endif
----------------
Quuxplusone wrote:
> This PR currently doesn't demonstrate any uses of `ASSERT_WITH_OPERATOR_NEW_FALLBACKS`.
> 
> I think you should go ahead and merge D107124 and D105910's changes into this PR, and then ask @NancyWang2222 and @DanielMcIntosh-IBM to download your patch and test it locally.
Oh, oops. The patch does cover the tests for both of them, but I accidentally used the wrong macro for the new/delete cases. I’ll revise the patch in 4-5 hours.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107755