[libcxx-commits] [libcxx] [libcxx] [test] Use ASSERT_WITH_LIBRARY_INTERNAL_ALLOCATIONS in more places (PR #144339)

[Martin Storsjö via libcxx-commits]

mstorsjo wrote:

> I'm cool disabling these for the filesystem tests at least, because there are likely `std::string`'s being constructed within the dylib in some hard-to-avoid manner.

It's not so much about strings constructed in the dylib; it's about the test code constructing code, but the compiler deciding on a case by case basis to either instantiate the ctors/dtors or call e.g. `_ZNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEED1Ev` from the DLL (which won't end up calling the overridden operator delete).

> The vector tests seem much more suspect. Do we have instantiations of `vector` in the dylib (on purpose)?

These tests do `std::vector<std::string>`.

> I acknowledge that the limitations exist. My concern is whether disabling _all_ allocation tests is the appropriate hammer, since not all "allocations under test" occur in the dylib, and in cases where they don't, they can likely work around the inconsistencies.

It's indeed a rather big hammer. However we already have the vast majority of those allocation tests already disabled through `ASSERT_WITH_LIBRARY_INTERNAL_ALLOCATIONS` (try grepping around the testsuite for this macro); this seems to be just a small number of more cases that gets weren't ignored so far.

And while disabling tests like that does lose some coverage, this macro only skips things for windows-dll cases. We still have `clang-cl-static` and `mingw-static` test configurations, where this macro doesn't skip anything and we still have full test coverage of the allocations on Windows.

https://github.com/llvm/llvm-project/pull/144339