[libcxx-commits] [PATCH] D82490: [libcxx] Put clang::trivial_abi on std::unique_ptr

[Vy Nguyen via Phabricator via libcxx-commits]

oontvoo added inline comments.


================
Comment at: libcxx/test/libcxx/memory/trivial_abi/unique_ptr_destruction_order.pass.cpp:13
+// Note: Unlike other tests in this directory, this is the only test that
+// exhibits a difference between the two modes in Microsft ABI.
+
----------------
mstorsjo wrote:
> @ontvoo - This doesn't seem to match actual observed behaviour here; now that we actually have Windows/MSVC CI, this test has ended up with an `XFAIL: LIBCXX-WINDOWS-FIXME`.
> 
> When I test running this testcase built with clang-cl, I get the same destruction order (A, B, C) regardless of whether the `_LIBCPP_ABI_ENABLE_UNIQUE_PTR_TRIVIAL_ABI` define is set or not.
> 
> Can you have a look and revisit the comment so it matches actual observed behaviour, and change the `XFAIL: LIBCXX-WINDOWS-FIXME` into something more specific?
Honestly I don't remember why I'd written this comment - maybe there was some subtlety there or maybe I was just being confused.

But yeah, in retrospect now, the destruction order should be the same (A,B,C) for MSVC regardless.
I'll update the tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82490