Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

storage_for missing from single include #1800

Closed
Tridacnid opened this issue Nov 7, 2019 · 3 comments
Closed

storage_for missing from single include #1800

Tridacnid opened this issue Nov 7, 2019 · 3 comments
Labels

Comments

@Tridacnid
Copy link
Contributor

Tridacnid commented Nov 7, 2019

Describe the bug
single_include/catch2/catch.hpp appears to be missing some benchmarking symbols that appear in the docs. Others may also be missing.

Expected behavior
Compiling the example code should work:

TEST_CASE("BLAH")
{
    BENCHMARK_ADVANCED("construct")(Catch::Benchmark::Chronometer meter)
    {
        std::vector<Catch::Benchmark::storage_for<std::string>> storage(meter.runs());
        meter.measure([&](int i) { storage[i].construct("thing"); });
   }
}

Observed behavior
error: storage_for is not a member of Catch::Benchmark

Reproduction steps
Compile the following:

#define CATCH_CONFIG_MAIN
#define CATCH_CONFIG_ENABLE_BENCHMARKING

#include <catch2/catch.hpp>

#include <vector>
#include <string>

TEST_CASE("BLAH")
{
    BENCHMARK_ADVANCED("construct")(Catch::Benchmark::Chronometer meter)
    {
        std::vector<Catch::Benchmark::storage_for<std::string>> storage(meter.runs());
        meter.measure([&](int i) { storage[i].construct("thing"); });
   }
}

Platform information:

  • Catch version: v2.10.2
@Tridacnid
Copy link
Contributor Author

Looks like include/internal/benchmark/catch_constructor.hpp isn't ever referenced in the source code, so the generateSingleHeader.py script doesn't include it in the final product. I'm going to try to submit a patch for both the missing inclusion as well as add a warning to the script so that in the future this sort of problem will be more visible to the developer.

@horenmar
Copy link
Member

Yeah, this sort of missing functionality has happened couple of times before.

Improvements to the stitching script are always welcome, but for the benchmarking specifically I would suggest creating file named something like catch_benchmarking_all.hpp inside the benchmarking folder, and including that for stitching. I suspect that our users will want such a header for the v3 version where we intend to provide Catch2 as a regular library made from multiple headers/TUs.

@Tridacnid
Copy link
Contributor Author

Thanks! I'll try to put together a patch to warn about this sort of thing when running the script. Is it intentional that SelfTest/UsageTests/Benchmark.test.cpp doesn't always compile with CATCH_CONFIG_ENABLE_BENCHMARKING?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants