Merge pull request #7430 from ethereum/push_no_args

Add push() for dynamic storage arrays

Changelog.md

@@ -9,6 +9,7 @@ Breaking changes:
  * General: Disallow explicit conversions from external function types to ``address`` and add a member called ``address`` to them as replacement.
  * General: New reserved keywords: ``virtual``.
  * Standard JSON Interface: Add option to disable or choose hash method between IPFS and Swarm for the bytecode metadata.
+ * Syntax: ``push(element)`` for dynamic storage arrays do not return the new length anymore.
  * Type checker: Resulting type of exponentiation is equal to the type of the base. Also allow signed types for the base.
 
 
@@ -17,6 +18,8 @@ Language Features:
  * Allow underscores as delimiters in hex strings.
  * Allow explicit conversions from ``address`` to ``address payable`` via ``payable(...)``.
  * Introduce syntax for array slices and implement them for dynamic calldata arrays.
+ * Introduce ``push()`` for dynamic storage arrays. It returns a reference to the newly allocated element, if applicable.
+ * Modify ``push(element)`` for dynamic storage arrays such that it does not return the new length anymore.
 
 
 Compiler Features:

docs/060-breaking-changes.rst

@@ -19,6 +19,8 @@ This section lists purely syntactic changes that do not affect the behavior of e
 * Conversions from ``address`` to ``address payable`` are now possible via ``payable(x)``, where
   ``x`` must be of type ``address``.
 
+* Function ``push(value)`` for dynamic storage arrays do not return the new length anymore.
+
 * New reserved keywords: ``virtual``.
 
 Semantic Only Changes
@@ -45,6 +47,9 @@ This section gives detailed instructions on how to update prior code for every b
 
 * Change ``address(f)`` to ``f.address`` for ``f`` being of external function type.
 
+* Change ``uint length = array.push(value)`` to ``array.push(value);``. The new length can be
+  accessed via ``array.length``.
+
 Deprecated Elements
 ===================
 

docs/types/reference-types.rst

@@ -109,8 +109,9 @@ restrictions for types apply, in that mappings can only be stored in the
 It is possible to mark state variable arrays ``public`` and have Solidity create a :ref:`getter <visibility-and-getters>`.
 The numeric index becomes a required parameter for the getter.
 
-Accessing an array past its end causes a failing assertion. You can use the ``.push()`` method to append a new element at the end or assign to the ``.length`` :ref:`member <array-members>` to change the size (see below for caveats).
-method or increase the ``.length`` :ref:`member <array-members>` to add elements.
+Accessing an array past its end causes a failing assertion. Methods ``.push()`` and ``.push(value)`` can be used
+to append a new element at the end of the array, where ``.push()`` appends a zero-initialized element and returns
+a reference to it.
 
 ``bytes`` and ``strings`` as Arrays
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -221,7 +222,9 @@ Array Members
     removed elements. If you try to resize a non-dynamic array that isn't in
     storage, you receive a ``Value must be an lvalue`` error.
 **push**:
-     Dynamic storage arrays and ``bytes`` (not ``string``) have a member function called ``push`` that you can use to append an element at the end of the array. The element will be zero-initialised. The function returns the new length.
+     Dynamic storage arrays and ``bytes`` (not ``string``) have a member function called ``push`` that you can use to append an element at the end of the array.
+     If no argument is given, the element will be zero-initialised and a reference to the new element is returned.
+     If a value is given as argument, ``push`` returns nothing.
 **pop**:
      Dynamic storage arrays and ``bytes`` (not ``string``) have a member function called ``pop`` that you can use to remove an element from the end of the array. This also implicitly calls :ref:`delete<delete>` on the removed element.
 
@@ -315,7 +318,8 @@ Array Members
         }
 
         function addFlag(bool[2] memory flag) public returns (uint) {
-            return m_pairsOfFlags.push(flag);
+            m_pairsOfFlags.push(flag);
+            return m_pairsOfFlags.length;
         }
 
         function createMemoryArray(uint size) public pure returns (bytes memory) {

libsolidity/analysis/TypeChecker.cpp

@@ -1889,7 +1889,7 @@ bool TypeChecker::visit(FunctionCall const& _functionCall)
 	FunctionCallAnnotation& funcCallAnno = _functionCall.annotation();
 	FunctionTypePointer functionType = nullptr;
 
-	// Determine and assign function call kind, purity and function type for this FunctionCall node
+	// Determine and assign function call kind, lvalue, purity and function type for this FunctionCall node
 	switch (expressionType->category())
 	{
 	case Type::Category::Function:
@@ -1903,6 +1903,12 @@ bool TypeChecker::visit(FunctionCall const& _functionCall)
 			functionType &&
 			functionType->isPure();
 
+		if (
+			functionType->kind() == FunctionType::Kind::ArrayPush ||
+			functionType->kind() == FunctionType::Kind::ByteArrayPush
+		)
+			funcCallAnno.isLValue = functionType->parameterTypes().empty();
+
 		break;
 
 	case Type::Category::TypeType:
@@ -2625,7 +2631,10 @@ void TypeChecker::requireLValue(Expression const& _expression)
 					return "Calldata structs are read-only.";
 			}
 			else if (auto arrayType = dynamic_cast<ArrayType const*>(type(memberAccess->expression())))
-				if (memberAccess->memberName() == "length")
+				if (
+					memberAccess->memberName() == "length" ||
+					memberAccess->memberName() == "push"
+				)
 					switch (arrayType->location())
 					{
 						case DataLocation::Memory:

libsolidity/ast/Types.cpp

@@ -1781,10 +1781,17 @@ MemberList::MemberMap ArrayType::nativeMembers(ContractDefinition const*) const
 		if (isDynamicallySized() && location() == DataLocation::Storage)
 		{
 			members.emplace_back("push", TypeProvider::function(
+				TypePointers{},
 				TypePointers{baseType()},
-				TypePointers{TypeProvider::uint256()},
+				strings{},
 				strings{string()},
+				isByteArray() ? FunctionType::Kind::ByteArrayPush : FunctionType::Kind::ArrayPush
+			));
+			members.emplace_back("push", TypeProvider::function(
+				TypePointers{baseType()},
+				TypePointers{},
 				strings{string()},
+				strings{},
 				isByteArray() ? FunctionType::Kind::ByteArrayPush : FunctionType::Kind::ArrayPush
 			));
 			members.emplace_back("pop", TypeProvider::function(

libsolidity/codegen/ExpressionCompiler.cpp

@@ -842,41 +842,66 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 		case FunctionType::Kind::ArrayPush:
 		{
 			_functionCall.expression().accept(*this);
-			solAssert(function.parameterTypes().size() == 1, "");
-			solAssert(!!function.parameterTypes()[0], "");
-			TypePointer paramType = function.parameterTypes()[0];
-			ArrayType const* arrayType =
-				function.kind() == FunctionType::Kind::ArrayPush ?
-				TypeProvider::array(DataLocation::Storage, paramType) :
-				TypeProvider::array(DataLocation::Storage);
 
-			// stack: ArrayReference
-			arguments[0]->accept(*this);
-			TypePointer const& argType = arguments[0]->annotation().type;
-			// stack: ArrayReference argValue
-			utils().moveToStackTop(argType->sizeOnStack(), 1);
-			// stack: argValue ArrayReference
-			m_context << Instruction::DUP1;
-			ArrayUtils(m_context).incrementDynamicArraySize(*arrayType);
-			// stack: argValue ArrayReference newLength
-			m_context << Instruction::SWAP1;
-			// stack: argValue newLength ArrayReference
-			m_context << u256(1) << Instruction::DUP3 << Instruction::SUB;
-			// stack: argValue newLength ArrayReference (newLength-1)
-			ArrayUtils(m_context).accessIndex(*arrayType, false);
-			// stack: argValue newLength storageSlot slotOffset
-			utils().moveToStackTop(3, argType->sizeOnStack());
-			// stack: newLength storageSlot slotOffset argValue
-			TypePointer type = arguments[0]->annotation().type->closestTemporaryType(arrayType->baseType());
-			solAssert(type, "");
-			utils().convertType(*argType, *type);
-			utils().moveToStackTop(1 + type->sizeOnStack());
-			utils().moveToStackTop(1 + type->sizeOnStack());
-			// stack: newLength argValue storageSlot slotOffset
-			if (function.kind() == FunctionType::Kind::ArrayPush)
-				StorageItem(m_context, *paramType).storeValue(*type, _functionCall.location(), true);
+			if (function.parameterTypes().size() == 0)
+			{
+				auto paramType = function.returnParameterTypes().at(0);
+				solAssert(paramType, "");
+
+				ArrayType const* arrayType =
+					function.kind() == FunctionType::Kind::ArrayPush ?
+					TypeProvider::array(DataLocation::Storage, paramType) :
+					TypeProvider::bytesStorage();
+
+				// stack: ArrayReference
+				m_context << u256(1) << Instruction::DUP2;
+				ArrayUtils(m_context).incrementDynamicArraySize(*arrayType);
+				// stack: ArrayReference 1 newLength
+				m_context << Instruction::SUB;
+				// stack: ArrayReference (newLength-1)
+				ArrayUtils(m_context).accessIndex(*arrayType, false);
+
+				if (arrayType->isByteArray())
+					setLValue<StorageByteArrayElement>(_functionCall);
+				else
+					setLValueToStorageItem(_functionCall);
+			}
 			else
-				StorageByteArrayElement(m_context).storeValue(*type, _functionCall.location(), true);
+			{
+				solAssert(function.parameterTypes().size() == 1, "");
+				solAssert(!!function.parameterTypes()[0], "");
+				TypePointer paramType = function.parameterTypes()[0];
+				ArrayType const* arrayType =
+					function.kind() == FunctionType::Kind::ArrayPush ?
+					TypeProvider::array(DataLocation::Storage, paramType) :
+					TypeProvider::bytesStorage();
+
+				// stack: ArrayReference
+				arguments[0]->accept(*this);
+				TypePointer const& argType = arguments[0]->annotation().type;
+				// stack: ArrayReference argValue
+				utils().moveToStackTop(argType->sizeOnStack(), 1);
+				// stack: argValue ArrayReference
+				m_context << Instruction::DUP1;
+				ArrayUtils(m_context).incrementDynamicArraySize(*arrayType);
+				// stack: argValue ArrayReference newLength
+				m_context << u256(1) << Instruction::SWAP1 << Instruction::SUB;
+				// stack: argValue ArrayReference (newLength-1)
+				ArrayUtils(m_context).accessIndex(*arrayType, false);
+				// stack: argValue storageSlot slotOffset
+				utils().moveToStackTop(2, argType->sizeOnStack());
+				// stack: storageSlot slotOffset argValue
+				TypePointer type = arguments[0]->annotation().type->closestTemporaryType(arrayType->baseType());
+				solAssert(type, "");
+				utils().convertType(*argType, *type);
+				utils().moveToStackTop(1 + type->sizeOnStack());
+				utils().moveToStackTop(1 + type->sizeOnStack());
+				// stack: argValue storageSlot slotOffset
+				if (function.kind() == FunctionType::Kind::ArrayPush)
+					StorageItem(m_context, *paramType).storeValue(*type, _functionCall.location(), true);
+				else
+					StorageByteArrayElement(m_context).storeValue(*type, _functionCall.location(), true);
+			}
 			break;
 		}
 		case FunctionType::Kind::ArrayPop:

test/compilationTests/corion/schelling.sol

@@ -70,7 +70,8 @@ contract schellingDB is safeMath, schellingVars {
         else { return (true, rounds[_id].totalAboveWeight, rounds[_id].totalBelowWeight, rounds[_id].reward, rounds[_id].blockHeight, rounds[_id].voted); }
     }
     function pushRound(uint256 _totalAboveWeight, uint256 _totalBelowWeight, uint256 _reward, uint256 _blockHeight, bool _voted) isOwner external returns(bool, uint256) {
-        return (true, rounds.push(_rounds(_totalAboveWeight, _totalBelowWeight, _reward, _blockHeight, _voted)));
+        rounds.push(_rounds(_totalAboveWeight, _totalBelowWeight, _reward, _blockHeight, _voted));
+        return (true, rounds.length);
     }
     function setRound(uint256 _id, uint256 _totalAboveWeight, uint256 _totalBelowWeight, uint256 _reward, uint256 _blockHeight, bool _voted) isOwner external returns(bool) {
         rounds[_id] = _rounds(_totalAboveWeight, _totalBelowWeight, _reward, _blockHeight, _voted);

test/libsolidity/SolidityEndToEndTest.cpp

@@ -4759,7 +4759,8 @@ BOOST_AUTO_TEST_CASE(array_push)
 				x = data[0];
 				data.push(4);
 				y = data[1];
-				l = data.push(3);
+				data.push(3);
+				l = data.length;
 				z = data[2];
 			}
 		}
@@ -4816,11 +4817,13 @@ BOOST_AUTO_TEST_CASE(byte_array_push)
 		contract c {
 			bytes data;
 			function test() public returns (bool x) {
-				if (data.push(0x05) != 1)  return true;
+				data.push(0x05);
+				if (data.length != 1)  return true;
 				if (data[0] != 0x05) return true;
 				data.push(0x04);
 				if (data[1] != 0x04) return true;
-				uint l = data.push(0x03);
+				data.push(0x03);
+				uint l = data.length;
 				if (data[2] != 0x03) return true;
 				if (l != 0x03) return true;
 			}
@@ -4860,7 +4863,8 @@ BOOST_AUTO_TEST_CASE(array_pop)
 			uint[] data;
 			function test() public returns (uint x, uint l) {
 				data.push(7);
-				x = data.push(3);
+				data.push(3);
+				x = data.length;
 				data.pop();
 				x = data.length;
 				data.pop();
@@ -4996,10 +5000,12 @@ BOOST_AUTO_TEST_CASE(byte_array_pop)
 			bytes data;
 			function test() public returns (uint x, uint y, uint l) {
 				data.push(0x07);
-				x = data.push(0x03);
+				data.push(0x03);
+				x = data.length;
 				data.pop();
 				data.pop();
-				y = data.push(0x02);
+				data.push(0x02);
+				y = data.length;
 				l = data.length;
 			}
 		}

test/libsolidity/semanticTests/array/push_no_args_1d.sol

@@ -0,0 +1,32 @@
+contract C {
+	uint[] array;
+
+	function f() public returns (uint) {
+		uint y = array.push();
+		return y;
+	}
+
+	function lv(uint value) public {
+		array.push() = value;
+	}
+
+	function a(uint index) public view returns (uint) {
+		return array[index];
+	}
+
+	function l() public view returns (uint) {
+		return array.length;
+	}
+
+}
+// ----
+// l() -> 0
+// lv(uint256): 42 ->
+// l() -> 1
+// a(uint256): 0 -> 42
+// f() -> 0
+// l() -> 2
+// a(uint256): 1 -> 0
+// lv(uint256): 111 ->
+// l() -> 3
+// a(uint256): 2 -> 111

test/libsolidity/semanticTests/array/push_no_args_2d.sol

@@ -0,0 +1,39 @@
+contract C {
+	uint[][] array2d;
+
+	function l() public returns (uint) {
+		return array2d.length;
+	}
+
+	function ll(uint index) public returns (uint) {
+		return array2d[index].length;
+	}
+
+	function a(uint i, uint j) public returns (uint) {
+		return array2d[i][j];
+	}
+
+	function f(uint index, uint value) public {
+		uint[] storage pointer = array2d.push();
+		for (uint i = 0; i <= index; ++i)
+			pointer.push();
+		pointer[index] = value;
+	}
+
+	function lv(uint value) public {
+		array2d.push().push() = value;
+	}
+}
+// ----
+// l() -> 0
+// f(uint256,uint256): 42, 64 ->
+// l() -> 1
+// ll(uint256): 0 -> 43
+// a(uint256,uint256): 0, 42 -> 64
+// f(uint256,uint256): 84, 128 ->
+// l() -> 2
+// ll(uint256): 1 -> 85
+// a(uint256,uint256): 0, 42 -> 64
+// a(uint256,uint256): 1, 84 -> 128
+// lv(uint256): 512 ->
+// a(uint256,uint256): 2, 0 -> 512

test/libsolidity/semanticTests/array/push_no_args_bytes.sol

@@ -0,0 +1,28 @@
+contract C {
+	bytes array;
+
+	function f() public {
+		array.push();
+	}
+
+	function g(uint x) public {
+		for (uint i = 0; i < x; ++i)
+			array.push() = bytes1(uint8(i));
+	}
+
+	function l() public returns (uint) {
+		return array.length;
+	}
+
+	function a(uint index) public view returns (bytes1) {
+		return array[index];
+	}
+}
+// ----
+// l() -> 0
+// g(uint256): 70 ->
+// l() -> 70
+// a(uint256): 69 -> left(69)
+// f() ->
+// l() -> 71
+// a(uint256): 70 -> 0

test/libsolidity/semanticTests/array/push_no_args_struct.sol

@@ -0,0 +1,44 @@
+contract C {
+	struct S {
+		uint x;
+	}
+
+	S[] array;
+
+	function f(uint y) public {
+		S storage s = array.push();
+		g(s, y);
+	}
+
+	function g(S storage s, uint y) internal {
+		s.x = y;
+	}
+
+	function h(uint y) public {
+		g(array.push(), y);
+	}
+
+	function lv(uint y) public {
+		array.push().x = y;
+	}
+
+	function a(uint i) public returns (uint) {
+		return array[i].x;
+	}
+
+	function l() public returns (uint) {
+		return array.length;
+	}
+
+}
+// ----
+// l() -> 0
+// f(uint256): 42 ->
+// l() -> 1
+// a(uint256): 0 -> 42
+// h(uint256): 84 ->
+// l() -> 2
+// a(uint256): 1 -> 84
+// lv(uint256): 4096 ->
+// l() -> 3
+// a(uint256): 2 -> 4096