From 1e3034a0eb009d65fc2184016619054af2625658 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Tue, 17 Aug 2021 18:59:57 +0200 Subject: [PATCH] Clarification with some comments and another assertion to help understanding the preconditions. --- libyul/backends/evm/StackHelpers.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/libyul/backends/evm/StackHelpers.h b/libyul/backends/evm/StackHelpers.h index 979177556..32e6119c4 100644 --- a/libyul/backends/evm/StackHelpers.h +++ b/libyul/backends/evm/StackHelpers.h @@ -107,8 +107,8 @@ class Shuffler public: /// Executes the stack shuffling operations. Instantiates an instance of ShuffleOperations /// in each iteration. Each iteration performs exactly one operation that modifies the stack. - /// After shuffle, source and target have the same size and all slots in the source layout are - /// compatible to the slots at the same target offset. + /// After `shuffle`, source and target have the same size and all slots in the source layout are + /// compatible with the slots at the same target offset. template static void shuffle(Args&&... args) { @@ -129,7 +129,7 @@ private: // Check if the stack is large enough for anything to potentially become unreachable. if (_ops.sourceSize() < 15) return false; - // Check whether any deep slot might still be needed later. + // Check whether any deep slot might still be needed later (i.e. we still need to reach it with a DUP or SWAP). for (size_t sourceOffset: ranges::views::iota(0u, _ops.sourceSize() - 15)) { // We need another copy of this slot. @@ -173,7 +173,7 @@ private: } return false; } - /// Finds a slot to dups or pushes with the aim of eventually fixing @a _targetOffset in the target. + /// Finds a slot to dup or push with the aim of eventually fixing @a _targetOffset in the target. /// In the simplest case, the slot at @a _targetOffset has a multiplicity > 0, i.e. it can directly be dupped or pushed /// and the next iteration will fix @a _targetOffset. /// But, in general, there may already be enough copies of the slot that is supposed to end up at @a _targetOffset @@ -197,7 +197,7 @@ private: _ops.pushOrDupTarget(offset); return true; } - // The desired target slot must already be somewhere else on stack right now. + // There must be another slot we can dup/push that will lead to the target slot at ``offset`` to be fixed. for (auto nextOffset: ranges::views::iota(0u, std::min(_ops.sourceSize(), _ops.targetSize()))) if ( !_ops.isCompatible(nextOffset, nextOffset) && @@ -258,6 +258,10 @@ private: return true; } + // ops.sourceSize() > ops.targetSize() cannot be true anymore, since if the source top is no longer required, + // we already popped it, and if it is required, we already swapped it down to a suitable target position. + yulAssert(ops.sourceSize() <= ops.targetSize(), ""); + // If a lower slot should be removed, try to bring up the slot that should end up there and bring it up. // Note that after the cases above, there will always be a target slot to duplicate in this case. for (size_t offset: ranges::views::iota(0u, ops.sourceSize()))