[libcxx-commits] [PATCH] D100029: [libcxx][test] Properly construct containers in debug mode tests for map/set

Kristina Bessonova via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Fri Apr 9 00:31:19 PDT 2021


krisb marked an inline comment as done.
krisb added a comment.

@curdeius thank you for looking at this!



================
Comment at: libcxx/test/libcxx/containers/unord/unord.multiset/db_iterators_7.pass.cpp:29-32
     ++i;
     assert(i == c.end());
     ++i;
     assert(false);
----------------
curdeius wrote:
> Quuxplusone wrote:
> > This PR seems like a step forward (especially once @curdeius' comments are addressed), but even after taking curdeius' comments and making the other tests match this one, don't we still have the same problem that if //any// libc++ assert //ever// fails, the test succeeds? So again here, if the `++i` on line 29 assert-fails (incorrectly), then the rest of the test doesn't run and the test appears to pass (incorrectly).
> > 
> > Scope creep!; but I wonder if these tests should be rewritten to use something like `#define _LIBCPP_ASSERT(x, m) ((x) ? (void)0 : foo(m))` and then define a `foo` that (1) checks for the exact message `m` that we expect, and (2) checks a global boolean that we set immediately before the operation that we expect to fail, so that "early failures" don't (incorrectly) count as successes.
> +1. Good remark.
@Quuxplusone good point, thank you! I'll take a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100029



More information about the libcxx-commits mailing list