Conversation
Previously, when we were trying to detect if all slots are reachable or final, we were checking only target slots that are filled at the current moment. However, we need to check *all* the target slots, even those beoynd the current stack size. Previous behaviour meant we were not shrinking the stack as much as was required in some situations and the shuffling went into a loop. With this fix most of the examples that were looping previously are now successful.
| { | ||
| // check that args are either in position or reachable | ||
| for (StackOffset const offset: _state.stackArgsRange()) | ||
| for (StackOffset offset{_state.target().tailSize}; offset < _state.target().size; ++offset.value) |
There was a problem hiding this comment.
I think we need a helper method for this. This is the second occurrence where we need this kind of iteration, which means it is time to put it in a helper with a proper name. Just not sure what would a good name actually be.
There was a problem hiding this comment.
what about argsRange in the State?
I.e. something along the lines of
auto argsRange() const
{
return ranges::views::iota(m_target.tailSize, m_stackData.size()) | ranges::views::transform([](auto _i) { return StackOffset{_i}; });
}There was a problem hiding this comment.
I don't think this would work. The point is that the range cannot be limited by the current stack data.
The needs to cover the target arg slot offsets.
There was a problem hiding this comment.
Yeah, sorry, I meant the iota range from tailSize to targetSize...
There was a problem hiding this comment.
Yes, that should work. And we can call it targetArgsRange, maybe?
2 participants
Previously, when we were trying to detect if all slots are reachable or final, we were checking only target slots that are filled at the current moment.
However, we need to check all the target slots, even those beoynd the current stack size.
Previous behaviour meant we were not shrinking the stack as much as was required in some situations and the shuffling went into a loop.
With this fix most of the examples that were looping previously are now successful.