Some more comments.

4 files changed, 25 insertions, 10 deletions

diff --git a/libjulia/backends/evm/AbstractAssembly.h b/libjulia/backends/evm/AbstractAssembly.h
index 73c6699f..baed833b 100644
--- a/libjulia/backends/evm/AbstractAssembly.h
+++ b/libjulia/backends/evm/AbstractAssembly.h
@@ -72,17 +72,22 @@ public:
 
 	/// Append a jump instruction.
 	/// @param _stackDiffAfter the stack adjustment after this instruction.
+	/// This is helpful to stack height analysis if there is no continuing control flow.
 	virtual void appendJump(int _stackDiffAfter) = 0;
 
 	/// Append a jump-to-immediate operation.
+	/// @param _stackDiffAfter the stack adjustment after this instruction.
 	virtual void appendJumpTo(LabelID _label, int _stackDiffAfter = 0) = 0;
 	/// Append a jump-to-if-immediate operation.
 	virtual void appendJumpToIf(LabelID _label) = 0;
-	/// Start a subroutine.
+	/// Start a subroutine identified by @a _label that takes @a _arguments
+	/// stack slots as arguments.
 	virtual void appendBeginsub(LabelID _label, int _arguments) = 0;
-	/// Call a subroutine.
+	/// Call a subroutine identified by @a _label, taking @a _arguments from the
+	/// stack upon call and putting @a _returns arguments onto the stack upon return.
 	virtual void appendJumpsub(LabelID _label, int _arguments, int _returns) = 0;
 	/// Return from a subroutine.
+	/// @param _stackDiffAfter the stack adjustment after this instruction.
 	virtual void appendReturnsub(int _returns, int _stackDiffAfter = 0) = 0;
 };
 
diff --git a/libjulia/backends/evm/EVMAssembly.cpp b/libjulia/backends/evm/EVMAssembly.cpp
index a24d9779..08702d2b 100644
--- a/libjulia/backends/evm/EVMAssembly.cpp
+++ b/libjulia/backends/evm/EVMAssembly.cpp
@@ -86,7 +86,7 @@ void EVMAssembly::appendJump(int _stackDiffAfter)
 	m_stackHeight += _stackDiffAfter;
 }
 
-void EVMAssembly::appendJumpTo(AbstractAssembly::LabelID _labelId, int _stackDiffAfter)
+void EVMAssembly::appendJumpTo(LabelID _labelId, int _stackDiffAfter)
 {
 	if (m_evm15)
 	{
@@ -101,7 +101,7 @@ void EVMAssembly::appendJumpTo(AbstractAssembly::LabelID _labelId, int _stackDif
 	}
 }
 
-void EVMAssembly::appendJumpToIf(AbstractAssembly::LabelID _labelId)
+void EVMAssembly::appendJumpToIf(LabelID _labelId)
 {
 	if (m_evm15)
 	{
@@ -116,7 +116,7 @@ void EVMAssembly::appendJumpToIf(AbstractAssembly::LabelID _labelId)
 	}
 }
 
-void EVMAssembly::appendBeginsub(AbstractAssembly::LabelID _labelId, int _arguments)
+void EVMAssembly::appendBeginsub(LabelID _labelId, int _arguments)
 {
 	solAssert(m_evm15, "BEGINSUB used for EVM 1.0");
 	solAssert(_arguments >= 0, "");
@@ -125,7 +125,7 @@ void EVMAssembly::appendBeginsub(AbstractAssembly::LabelID _labelId, int _argume
 	m_stackHeight += _arguments;
 }
 
-void EVMAssembly::appendJumpsub(AbstractAssembly::LabelID _labelId, int _arguments, int _returns)
+void EVMAssembly::appendJumpsub(LabelID _labelId, int _arguments, int _returns)
 {
 	solAssert(m_evm15, "JUMPSUB used for EVM 1.0");
 	solAssert(_arguments >= 0 && _returns >= 0, "");
@@ -160,14 +160,14 @@ eth::LinkerObject EVMAssembly::finalize()
 	return obj;
 }
 
-void EVMAssembly::setLabelToCurrentPosition(AbstractAssembly::LabelID _labelId)
+void EVMAssembly::setLabelToCurrentPosition(LabelID _labelId)
 {
 	solAssert(m_labelPositions.count(_labelId), "Label not found.");
 	solAssert(m_labelPositions[_labelId] == size_t(-1), "Label already set.");
 	m_labelPositions[_labelId] = m_bytecode.size();
 }
 
-void EVMAssembly::appendLabelReferenceInternal(AbstractAssembly::LabelID _labelId)
+void EVMAssembly::appendLabelReferenceInternal(LabelID _labelId)
 {
 	m_labelReferences[m_bytecode.size()] = _labelId;
 	m_bytecode += bytes(labelReferenceSize);
diff --git a/libjulia/backends/evm/EVMCodeTransform.cpp b/libjulia/backends/evm/EVMCodeTransform.cpp
index 43bd5a44..2b6a11e0 100644
--- a/libjulia/backends/evm/EVMCodeTransform.cpp
+++ b/libjulia/backends/evm/EVMCodeTransform.cpp
@@ -262,6 +262,7 @@ void CodeTransform::operator()(Switch const& _switch)
 		m_assembly.setSourceLocation(c.first->location);
 		m_assembly.appendLabel(c.second);
 		(*this)(c.first->body);
+		// Avoid useless "jump to next" for the last case.
 		if (--numCases > 0)
 		{
 			m_assembly.setSourceLocation(c.first->location);
@@ -315,6 +316,7 @@ void CodeTransform::operator()(FunctionDefinition const& _function)
 		auto& var = boost::get<Scope::Variable>(varScope->identifiers.at(v.name));
 		var.stackHeight = height++;
 		var.active = true;
+		// Preset stack slots for return variables to zero.
 		m_assembly.appendConstant(u256(0));
 	}
 
@@ -322,13 +324,20 @@ void CodeTransform::operator()(FunctionDefinition const& _function)
 		.run(_function.body);
 
 	{
-		// Stack of target positions of stack elements
+		// The stack layout here is:
+		// <return label>? <arguments...> <return values...>
+		// But we would like it to be:
+		// <return values...> <return label>?
+		// So we have to append some SWAP and POP instructions.
+
+		// This vector holds the desired target positions of all stack slots and is
+		// modified parallel to the actual stack.
 		vector<int> stackLayout;
 		if (!m_evm15)
 			stackLayout.push_back(_function.returns.size()); // Move return label to the top
 		stackLayout += vector<int>(_function.arguments.size(), -1); // discard all arguments
 		for (size_t i = 0; i < _function.returns.size(); ++i)
-			stackLayout.push_back(i);
+			stackLayout.push_back(i); // Move return values down, but keep order.
 
 		solAssert(stackLayout.size() <= 17, "Stack too deep");
 		while (!stackLayout.empty() && stackLayout.back() != int(stackLayout.size() - 1))
diff --git a/libjulia/backends/evm/EVMCodeTransform.h b/libjulia/backends/evm/EVMCodeTransform.h
index a817efea..39f542e3 100644
--- a/libjulia/backends/evm/EVMCodeTransform.h
+++ b/libjulia/backends/evm/EVMCodeTransform.h
@@ -104,6 +104,7 @@ public:
 	void operator()(solidity::assembly::FunctionDefinition const&);
 	void operator()(solidity::assembly::Block const& _block);
 
+private:
 	AbstractAssembly::LabelID labelFromIdentifier(solidity::assembly::Identifier const& _identifier);
 	/// Generates code for an expression that is supposed to return a single value.
 	void visitExpression(solidity::assembly::Statement const& _expression);