[libcxx-commits] [PATCH] D100595: [libcxx][test] Attempt to make debug mode tests more bulletproof

Tue Apr 20 04:16:41 PDT 2021

curdeius added inline comments.

================
Comment at: libcxx/test/libcxx/containers/sequences/list/list.cons/db_move.pass.cpp:21

 #include <list>
 #include <cstdlib>
----------------
krisb wrote:
> ldionne wrote:
> > curdeius wrote:
> > > It seems to me that other headers are unnecessary here.
> > Now this makes more sense to me - we just don't want to overwrite the `_LIBCPP_ASSERT` macro.
> > 
> > However, shouldn't we now rename this test since it's not related to the libc++ debug mode anymore? And while we're at it, maybe we don't need this test anymore if it's already tested in the tests for `std::list`?
> It doesn't seem like we have some normal mode tests that check that iterators keep valid after moving. But I think it can still be valuable in debug mode (I mean with `-D_LIBCPP_DEBUG=1` defined) as it also checks that debug mode correctly handles cases like this and doesn't trigger any assertions (which would be triggered, for example, if there were two different containers). Does it make sense?
> 
I think it's important to keep these tests with `-D_LIBCPP_DEBUG=1/2` as @krisb indicated because they should pass in debug mode. However, as their purpose isn't clear, it would be wise to add a comment.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100595