Tests for Clang-tidy STL containers checks

Hi!

While working on LLDB code cleanup, I discovered that some of
Clang-tidy STL containers checks don't work properly with LLVM STL
implementation. More interesting discovery was that actual STL
containers were not used in tests, but were replaced with code
snippets.

I think proper solution will be to use real STL headers and pass
proper compiler flags from CMAKE_CXX_FLAGS when compiled with Clang
and create -stdlib=libstdc++ --gcc-toolchain=<path> when compiled with
GCC. Similar solution could be implemented for other compilers/STL
implementations.

Eugene.

We mock out STL containers for multiple reasons. One of them is to make the tests hermetic (as in “independent from the environment”). The developer has to make the mocks appropriately model the implementation of the standard library. If another implementation is sufficiently different, we need to add mocks for that implementation as well. Making the tests use actual library headers is problematic, since it will significantly increase testing time and complicate integration with different tests environments. So I’m strongly against this. We could make a mode where the actual headers are used instead of mock implementations, but we’re not going to remove mocks completely.

Please file bugs for the specific issues you find. If you can isolate specific differences between library implementations that result in incorrect work of the checks (and maybe create the appropriate mocks for the library you use), it would help check developers to fix their checks.

Hi, Alexander!

We mock out STL containers for multiple reasons. One of them is to make the
tests hermetic (as in "independent from the environment"). The developer has
to make the mocks appropriately model the implementation of the standard
library. If another implementation is sufficiently different, we need to add
mocks for that implementation as well. Making the tests use actual library
headers is problematic, since it will significantly increase testing time
and complicate integration with different tests environments. So I'm
strongly against this. We could make a mode where the actual headers are
used instead of mock implementations, but we're not going to remove mocks
completely.

Problem with hermetic tests that they are testing mocks, not real
implementations. I caught problems manually because I was lucky to
spot pattern which was supposed to be recognized by
readability-container-size-empty, but it was not. But this should be
caught by regular automated regressions to maintain reliability of
Clang-tidy checks.

I don't think that including STL headers will increase tests time significantly.

Please file bugs for the specific issues you find. If you can isolate
specific differences between library implementations that result in
incorrect work of the checks (and maybe create the appropriate mocks for the
library you use), it would help check developers to fix their checks.

I already filed bugs 25804, 25812 and 25813 and developers took care
about them, through problems were not fixed completely yet.

Eugene.

Umm, no.

IIUC, these are tests for Clang-tidy, so they test the behavior of
Clang-tidy code.
STL containers are NOT under test; this is why they are mocked out.

Csaba

Hi, Alexander!

We mock out STL containers for multiple reasons. One of them is to make the
tests hermetic (as in "independent from the environment"). The developer has
to make the mocks appropriately model the implementation of the standard
library. If another implementation is sufficiently different, we need to add
mocks for that implementation as well. Making the tests use actual library
headers is problematic, since it will significantly increase testing time
and complicate integration with different tests environments. So I'm
strongly against this. We could make a mode where the actual headers are
used instead of mock implementations, but we're not going to remove mocks
completely.

Problem with hermetic tests that they are testing mocks, not real
implementations. I caught problems manually because I was lucky to
spot pattern which was supposed to be recognized by
readability-container-size-empty, but it was not. But this should be
caught by regular automated regressions to maintain reliability of
Clang-tidy checks.

I don't think that including STL headers will increase tests time significantly.

I would be opposed to requiring a "real" STL implementation as well,
though I don't worry about test time. For instance, MSVC 2015 STL
headers require also passing -fms-compatibility-version=19, which does
not happen automatically (we default to an older version), and so you
will get compile errors. Using whatever STL implementation happens to
be on the developer's machine instead of a consistent STL mock means
that the tests will be unreliable by definition. You have no idea
*what* you are actually testing in that case.

~Aaron