doctest icon indicating copy to clipboard operation
doctest copied to clipboard

Can `doctest::String` be less required in the public API?

Open RedBeard0531 opened this issue 7 months ago • 3 comments

There is already DOCTEST_CONFIG_DOUBLE_STRINGIFY so that toString() implementations can use a normal string type. It would be nice if the rest of the API could be made to not depend on doctest::String as well. In particular, it would be nice if SUBCASE() took anything that could be passed to toString(). It would be even better if you could do for (auto x : someRange) { SUBCASE("x: ", x) {...} }, similar to MESSAGE("x: ", x).

In general I'm not sure that there is a ton of value in having your own String type at this point. It would be nice to at least offer some config option that just makes it a typedef for std::string. Modern compilers can include <string> fairly quickly (<100ms on my laptop), so avoiding it isn't saving that much time. But also basically every TU (at least in our codebase) will have included it anyway, so defining your own is more rather than less code for the compiler to deal with.

RedBeard0531 avatar May 16 '25 15:05 RedBeard0531

I agree that an optional typedef to std::string would be handy, but the core design decision of doctest is speed, and that's usually at odds with stdlib headers.

In my view, it's easier to add in converting operators from string-like types to doctest::String, but this could be looked into more.

mitchgrout avatar May 16 '25 21:05 mitchgrout

FWIW, compilers have gotten a lot better at compiling std headers than they used to be. On my laptop, g++ 14.2 and clang 19.1 each take about 35ms to compile string_view and about 70ms to compile string (which transitively includes string_view). That is at -std=c++17 which lets it take advantage of these extern template declarations. At -std=c++20, the numbers are a bit worse at ~60ms and ~100ms, but still quite low in an absolute sense. For comparison doctest.h takes about 30ms by itself , 45ms with string_view and 80 with string (all in C++17).

I recognize that going from 30ms to 80ms is slower, but I don't think most people will mind (or even notice) an extra 50ms per TU. That seems like a good tradeoff for the maintainability of the project.

My timings were using commands like time clang++ -std=c++17 -include 'string' -include 'doctest.h' -x c++ -c /dev/null -o /dev/null if you would like to reproduce them.

RedBeard0531 avatar May 27 '25 12:05 RedBeard0531

FWIW, compilers have gotten a lot better at compiling std headers than they used to be. On my laptop, g++ 14.2 and clang 19.1 each take about 35ms to compile string_view and about 70ms to compile string (which transitively includes string_view). That is at -std=c++17 which lets it take advantage of these extern template declarations. At -std=c++20, the numbers are a bit worse at ~60ms and ~100ms, but still quite low in an absolute sense. For comparison doctest.h takes about 30ms by itself , 45ms with string_view and 80 with string (all in C++17).

I recognize that going from 30ms to 80ms is slower, but I don't think most people will mind (or even notice) an extra 50ms per TU. That seems like a good tradeoff for the maintainability of the project.

My timings were using commands like time clang++ -std=c++17 -include 'string' -include 'doctest.h' -x c++ -c /dev/null -o /dev/null if you would like to reproduce them.

An unconditional change from 30ms to 80ms is frankly not acceptable for larger projects, especially since doctest shines in compilation speed. In one project, I have about 500 tests, of which very few use <string> or <string_view> even indirectly; I would rather avoid eating a 25s+ user-time cost if I can help it.

Obviously this means I'm biased against having <string> be unconditionally present; but as you mentioned originally, a config similar to INCLUDE_TYPE_TRAITS is doable -- however, we'd want to be careful that adding this config option doesn't cause the doctest::String and std::string capabilities to diverge too far.

mitchgrout avatar May 27 '25 21:05 mitchgrout