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

[Kristina Bessonova via Phabricator via libcxx-commits]

krisb added inline comments.


================
Comment at: libcxx/test/libcxx/containers/unord/unord.map/db_local_iterators_7.pass.cpp:16
 // ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DEBUG=1
 #define _LIBCPP_ASSERT(x, m) ((x) ? (void)0 : std::exit(0))
 
----------------
curdeius wrote:
> To answer @Quuxplusone's point 2), you can do just something like the suggested edits here... (see another comment for the rest)
> 
> Comparing the assertion messages may be a bit difficult though, and brittle.
> 
> I.e. if we wanted to do something like:
> ```
> #include <assert.h>
> #include <stdlib.h>
> #include <string.h>
> 
> void test_assertion(const char* msg) {
>     assert(::strcmp(msg, "expected") == 0);
>     ::exit(::test_exit_code);
> }
> 
> #define _LIBCPP_ASSERT(x, m) ((x) ? (void)0 : ::test_assertion(m))
> ```
> 
> we hit the problem that we include some headers for assert, exit and strcmp, but we redefine `_LIBCPP_ASSERT` afterwards.
> Also these headers might get included transitively by later includes (but won't be because of header guards), but they'll have `_LIBCPP_ASSERT` not redefined.
> 
> An orthogonal problem is to have the actual message when `assert(strcmp(...)...);` fails.
>  we hit the problem that we include some headers for assert, exit and strcmp, but we redefine _LIBCPP_ASSERT afterwards

It doesn't seem like we need to care about `assert.h`, `stdlib.h` and other 'C'-headers here, since `_LIBCPP_ASSERT` isn't expected to appear in them and any includes.

So for the particular test it may look like:

```
#include <string.h>
#include <cassert>
int test_exit_code = 1;
#define _LIBCPP_ASSERT(x, m)                                                   \
  ((x) ? (void)0                                                               \
       : ((::strcmp(m, "Attempted to increment non-incrementable unordered "   \
                       "container local_iterator") == 0)                       \
              ? std::exit(::test_exit_code)                                    \
              : assert(x)))

```

and later in the test

```
int main(int, char**) {
    ...
    test_exit_code = 0;
    ++i; /* _LIBCPP_ASSERT is expected to fail here */
    ...
```
 I'll test the approach and create a separate review for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100029