New warnings with GCC 7.x

[dgeelen]

This fixes 2 new compiler warnings.

[dgeelen]

CORO_FORK is defined as:

#define CORO_FORK CORO_FORK_IMPL(__COUNTER__ - _coro_counter_base + 1)
#define CORO_FORK_IMPL(n) \
  for (_coro_value = (n);; _coro_value = -1) \
    if (_coro_value == -1) \
    { \
      case (n): ; \
      break; \
    } \
    else

So the warning was for the else inside the macro possibly belonging to the outermost if (!execwait) instead of the inner if (_coro_value == -1). The intention here is to fork only when !execwait, so the added braces capture this intention.

[dgeelen]

The for/break combination in the CORO_FORK macro enables skipping over the user statement following the fork when resuming. For this to work, we need to somehow make the user code part of the for loop.

We want the user statement following CORO_FORK to be executed exactly once, the first time that this code is executed (i.e. before we 're-enter'). A possible alternative solution is this (compiles, but otherwise not tested yet):

#define CORO_FORK_IMPL(n) \
  for (_coro_value = -1; _coro_value != (n);) \
    case (n): for (; _coro_value != (n); _coro_value = (n))

This works as follows. When re-entering, _coro_value is set to the continuation point, and used to 'switch' to the correct location (e.g. the case (n): above). So we know that when re-entering through the case statement _coro_value == n, and we should not execute the user statement. This is achieved by using an initially false statement for the loop guard of the second loop (in which the user statement will be contained). In order to execute the inner loop once, the initial condition needs to be true, i.e. _coro_value != (n). A good default is to use -1, which is never a valid value for _coro_value. Now the inner loop's condition holds and the loop will be executed once, because after one iteration the inner loop will set _coro_value to (n), terminating both loops. A potential issue with the above is that in the original code we have:

• After initial execution, _coro_value is -1
• After resuming, _coro_value is (n)

While in the new code _coro_value is always (n).

Additionally, this solves the problem only for CORO_FORK, but it looks to me like the other macros (yield and reenter) suffer from the same problem. Fixing those looks... more complicated.

[MrHacky]

After reading through coroutine.hpp again, i think the difference in _coro_value will actually indeed be a problem. See line 18: bool is_parent() const { return value_ < 0; } The value being -1 or not is actually used here to be able to determine in which branch of the fork the current context is running.

In addition, _coro_value with your new code will have a different value during execution of the body:

• Original code the value will be n during execution of the statement after the last else
• New code the value will be -1 during execution of the statement after the final for.

This is actually important as well as this value will be copied to remember the current state, and will be used in the coro switch statement to resume at the case (n): next time.

Furthermore, in line 33: int& operator=(int v) { modified_ = true; return value_ = v; } we can see assigment into _coro_value actually has the sideeffect of setting modified_ to true. Which is something we have to avoid in the case of resuming at the case (n): label. (your new code actually still does this correctly)

And indeed this is only considering CORO_FORK which in comparison with CORO_YIELD actually seems pretty simple...