Implements pop() for value type arrays.

6 files changed, 141 insertions, 1 deletions

diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp
index 59668e1d..1c6003cd 100644
--- a/libsolidity/ast/Types.cpp
+++ b/libsolidity/ast/Types.cpp
@@ -1689,6 +1689,7 @@ MemberList::MemberMap ArrayType::nativeMembers(ContractDefinition const*) const
 	{
 		members.push_back({"length", make_shared<IntegerType>(256)});
 		if (isDynamicallySized() && location() == DataLocation::Storage)
+		{
 			members.push_back({"push", make_shared<FunctionType>(
 				TypePointers{baseType()},
 				TypePointers{make_shared<IntegerType>(256)},
@@ -1696,6 +1697,14 @@ MemberList::MemberMap ArrayType::nativeMembers(ContractDefinition const*) const
 				strings{string()},
 				isByteArray() ? FunctionType::Kind::ByteArrayPush : FunctionType::Kind::ArrayPush
 			)});
+			members.push_back({"pop", make_shared<FunctionType>(
+				TypePointers{},
+				TypePointers{},
+				strings{string()},
+				strings{string()},
+				isByteArray() ? FunctionType::Kind::ByteArrayPop : FunctionType::Kind::ArrayPop
+			)});
+		}
 	}
 	return members;
 }
diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h
index 7c6f179b..29c7db86 100644
--- a/libsolidity/ast/Types.h
+++ b/libsolidity/ast/Types.h
@@ -914,7 +914,9 @@ public:
 		AddMod, ///< ADDMOD
 		MulMod, ///< MULMOD
 		ArrayPush, ///< .push() to a dynamically sized array in storage
+		ArrayPop, ///< .pop() from a dynamically sized array in storage
 		ByteArrayPush, ///< .push() to a dynamically sized byte array in storage
+		ByteArrayPop, ///< .pop() from a dynamically sized byte array in storage
 		ObjectCreation, ///< array creation using new
 		Assert, ///< assert()
 		Require, ///< require()
diff --git a/libsolidity/codegen/ArrayUtils.cpp b/libsolidity/codegen/ArrayUtils.cpp
index 0fe66d2d..b434fddd 100644
--- a/libsolidity/codegen/ArrayUtils.cpp
+++ b/libsolidity/codegen/ArrayUtils.cpp
@@ -823,6 +823,39 @@ void ArrayUtils::incrementDynamicArraySize(ArrayType const& _type) const
 		})", {"ref"});
 }
 
+void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const
+{
+	solAssert(_type.location() == DataLocation::Storage, "");
+	solAssert(_type.isDynamicallySized(), "");
+	if (!_type.isByteArray() && _type.baseType()->storageBytes() < 32)
+		solAssert(_type.baseType()->isValueType(), "Invalid storage size for non-value type.");
+
+	// stack: ArrayReference
+	retrieveLength(_type);
+	// stack: ArrayReference oldLength
+	m_context << Instruction::DUP1;
+	// stack: ArrayReference oldLength oldLength
+	m_context << Instruction::ISZERO;
+	m_context.appendConditionalInvalid();
+
+	if (_type.isByteArray())
+	{
+	}
+	else
+	{
+		// Stack: ArrayReference oldLength
+		m_context << u256(1) << Instruction::SWAP1 << Instruction::SUB;
+		// Stack ArrayReference newLength
+		m_context << Instruction::DUP2 << Instruction::DUP2;
+		// Stack ArrayReference newLength ArrayReference newLength;
+		accessIndex(_type, false);
+		// Stack: ArrayReference newLength storage_slot byte_offset
+		StorageItem(m_context, _type).setToZero(SourceLocation(), true);
+		// Stack: ArrayReference newLength
+		m_context << Instruction::SSTORE;
+	}
+}
+
 void ArrayUtils::clearStorageLoop(TypePointer const& _type) const
 {
 	m_context.callLowLevelFunction(
diff --git a/libsolidity/codegen/ArrayUtils.h b/libsolidity/codegen/ArrayUtils.h
index 99786397..84d591d7 100644
--- a/libsolidity/codegen/ArrayUtils.h
+++ b/libsolidity/codegen/ArrayUtils.h
@@ -73,6 +73,11 @@ public:
 	/// Stack pre: reference (excludes byte offset)
 	/// Stack post: new_length
 	void incrementDynamicArraySize(ArrayType const& _type) const;
+	/// Decrements the size of a dynamic array by one if length is nonzero. Returns otherwise.
+	/// Clears the removed data element. In case of a byte array, this might move the data.
+	/// Stack pre: reference
+	/// Stack post:
+	void popStorageArrayElement(ArrayType const& _type) const;
 	/// Appends a loop that clears a sequence of storage slots of the given type (excluding end).
 	/// Stack pre: end_ref start_ref
 	/// Stack post: end_ref
diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index 4bcc1fa9..ac7610fc 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -866,6 +866,20 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 				StorageByteArrayElement(m_context).storeValue(*type, _functionCall.location(), true);
 			break;
 		}
+		case FunctionType::Kind::ByteArrayPop:
+		case FunctionType::Kind::ArrayPop:
+		{
+			_functionCall.expression().accept(*this);
+			solAssert(function.parameterTypes().empty(), "");
+
+			ArrayType const& arrayType = dynamic_cast<ArrayType const&>(
+				*dynamic_cast<MemberAccess const&>(_functionCall.expression()).expression().annotation().type
+			);
+			solAssert(arrayType.dataStoredIn(DataLocation::Storage), "");
+
+			ArrayUtils(m_context).popStorageArrayElement(arrayType);
+			break;
+		}
 		case FunctionType::Kind::ObjectCreation:
 		{
 			ArrayType const& arrayType = dynamic_cast<ArrayType const&>(*_functionCall.annotation().type);
@@ -1348,10 +1362,21 @@ bool ExpressionCompiler::visit(MemberAccess const& _memberAccess)
 		else if (member == "push")
 		{
 			solAssert(
-				type.isDynamicallySized() && type.location() == DataLocation::Storage,
+				type.isDynamicallySized() &&
+				type.location() == DataLocation::Storage &&
+				type.category() == Type::Category::Array,
 				"Tried to use .push() on a non-dynamically sized array"
 			);
 		}
+		else if (member == "pop")
+		{
+			solAssert(
+				type.isDynamicallySized() &&
+				type.location() == DataLocation::Storage &&
+				type.category() == Type::Category::Array,
+				"Tried to use .pop() on a non-dynamically sized array"
+			);
+		}
 		else
 			solAssert(false, "Illegal array member.");
 		break;
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 40962294..b4cf6950 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -5111,6 +5111,72 @@ BOOST_AUTO_TEST_CASE(byte_array_push_transition)
 	ABI_CHECK(callContractFunction("test()"), encodeArgs(0));
 }
 
+BOOST_AUTO_TEST_CASE(array_pop)
+{
+	char const* sourceCode = R"(
+		contract c {
+			uint[] data;
+			function test() public returns (uint x, uint y, uint l) {
+				data.push(7);
+				x = data.push(3);
+				data.pop();
+				y = data.push(2);
+				l = data.length;
+			}
+		}
+	)";
+	compileAndRun(sourceCode);
+	ABI_CHECK(callContractFunction("test()"), encodeArgs(2, 2, 2));
+}
+
+BOOST_AUTO_TEST_CASE(array_pop_empty)
+{
+	char const* sourceCode = R"(
+		contract c {
+			uint[] data;
+			function test() public returns (bool) {
+				data.pop();
+				return true;
+			}
+		}
+	)";
+	compileAndRun(sourceCode);
+	ABI_CHECK(callContractFunction("test()"), encodeArgs());
+}
+
+BOOST_AUTO_TEST_CASE(bytearray_pop)
+{
+	char const* sourceCode = R"(
+		contract c {
+			bytes data;
+			function test() public returns (uint x, uint y, uint l) {
+				data.push(7);
+				x = data.push(3);
+				data.pop();
+				data.pop();
+				y = data.push(2);
+				l = data.length;
+			}
+		}
+	)";
+	compileAndRun(sourceCode);
+	ABI_CHECK(callContractFunction("test()"), encodeArgs(2, 1, 1));
+}
+
+BOOST_AUTO_TEST_CASE(bytearray_pop_empty)
+{
+	char const* sourceCode = R"(
+		contract c {
+			bytes data;
+			function test() public returns (bool) {
+				data.pop();
+			}
+		}
+	)";
+	compileAndRun(sourceCode);
+	ABI_CHECK(callContractFunction("test()"), encodeArgs());
+}
+
 BOOST_AUTO_TEST_CASE(external_array_args)
 {
 	char const* sourceCode = R"(