When the same SECTION is encountered multiple times in one cycle, catch crashes on assert

[mjerabek]

When the same SECTION is encountered multiple times in one cycle, catch crashes on assert

[mjerabek]

Note: this should hopefully be somewhat coherent, but I've been staring at the code for a few hours and it's late, so sorry if it isnt 😄

The problem

When the section is acquired the second time, we have already ctx.completedCycle(), so we don't call tryOpen(). But since the section isComplete(), close() is still called, leading to the crash, because we go past the root tracker.

So, the minimal solution is to not enter the section if ctx.completedCycle(). But after the "section" finished for the first time, it NeedsAnotherRun (because it has more subsections), and therefore isOpen() is true. And it generally cannot be false for NeedsAnotherRun, because we can enter such a section if !ctx.completedCycle() (TODO: really?).

So, could we just check ctx.completedCycle() in isOpen()? Well, no. Because all currently opened sections (with more following children) would suddenly become not opened.

The solution

So, we cannot hack it, we have to do it properly. The goal is to track if a section was already encountered during the current cycle and if so, it's an error.

I think the most robust solution is to remember the serial number of the cycle in each Tracker and in SectionTracker::acquire, if we find that the requested section's last recorded cycle is the current cycle, that's the error. (Side note: we don't have to worry about overflow, one-digit binary counter would be enough for this)

Extension of the solution

Alternatively, instead of erroring out, we can say "ok, let's make this a new section and make it work". That would mean that the section tracker tracks also "generation" in addition to name and location.

What about generators

I think it could work the same - if we encounter a generator a second time in the same cycle, we just say it's a different generator. Although I have not studied how generators are implemented yet - even understanding how sections work took me a few hours.

Why it might not be a good idea after all

The problem comes with reporting - how do we distinguish between the two identical sections? We don't. Hence, I think we should force the user to insert (dynamic) sections to make the sections unique.

How to report the error

Catch should probably just throw an exception. But could the malicious user just catch(...){} it? Probably yes. Am I getting overly paranoid? Definitely.

[mjerabek]

Another potentially interesting test case with generators:

TEST_CASE("foo") {
    std::cout << "new run\n";
    for (int i = 0; i < 3; ++i) {
        SECTION("section") {
            int n = GENERATE(1, 2);
            std::cout << n << "\n";
        }
    }
}

Edit: probably not interesting after all - it's the SECTION that will make it or break it. And generators directly in a loop are handled correctly now.