From 43d6e00b143f9f2fd83fbca3ab4bbd1d3b33bf83 Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Sat, 14 Sep 2019 00:54:51 +0200 Subject: [PATCH] Add push() for dynamic storage arrays --- Changelog.md | 3 + docs/060-breaking-changes.rst | 5 + docs/types/reference-types.rst | 12 ++- libsolidity/analysis/TypeChecker.cpp | 13 ++- libsolidity/ast/Types.cpp | 9 +- libsolidity/codegen/ExpressionCompiler.cpp | 91 ++++++++++++------- test/compilationTests/corion/schelling.sol | 3 +- test/libsolidity/SolidityEndToEndTest.cpp | 18 ++-- .../semanticTests/array/push_no_args_1d.sol | 32 +++++++ .../semanticTests/array/push_no_args_2d.sol | 39 ++++++++ .../array/push_no_args_bytes.sol | 28 ++++++ .../array/push_no_args_struct.sol | 44 +++++++++ 12 files changed, 250 insertions(+), 47 deletions(-) create mode 100644 test/libsolidity/semanticTests/array/push_no_args_1d.sol create mode 100644 test/libsolidity/semanticTests/array/push_no_args_2d.sol create mode 100644 test/libsolidity/semanticTests/array/push_no_args_bytes.sol create mode 100644 test/libsolidity/semanticTests/array/push_no_args_struct.sol diff --git a/Changelog.md b/Changelog.md index 680a02c08..45bc06e5e 100644 --- a/Changelog.md +++ b/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: diff --git a/docs/060-breaking-changes.rst b/docs/060-breaking-changes.rst index 79cc268de..effa966f9 100644 --- a/docs/060-breaking-changes.rst +++ b/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 =================== diff --git a/docs/types/reference-types.rst b/docs/types/reference-types.rst index de1e5f543..ed565f80a 100644 --- a/docs/types/reference-types.rst +++ b/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 `. 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 ` to change the size (see below for caveats). -method or increase the ``.length`` :ref:`member ` 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` 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) { diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 2ac071375..fcf81c2da 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/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(type(memberAccess->expression()))) - if (memberAccess->memberName() == "length") + if ( + memberAccess->memberName() == "length" || + memberAccess->memberName() == "push" + ) switch (arrayType->location()) { case DataLocation::Memory: diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index b509c2495..b9f9561cc 100644 --- a/libsolidity/ast/Types.cpp +++ b/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( diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index 87f1a9d39..57a0cb189 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/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(_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: diff --git a/test/compilationTests/corion/schelling.sol b/test/compilationTests/corion/schelling.sol index e70716bdf..9ab24133a 100644 --- a/test/compilationTests/corion/schelling.sol +++ b/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); diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 1e9ce251c..1c14b95a0 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/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; } } diff --git a/test/libsolidity/semanticTests/array/push_no_args_1d.sol b/test/libsolidity/semanticTests/array/push_no_args_1d.sol new file mode 100644 index 000000000..cf6f0687d --- /dev/null +++ b/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 diff --git a/test/libsolidity/semanticTests/array/push_no_args_2d.sol b/test/libsolidity/semanticTests/array/push_no_args_2d.sol new file mode 100644 index 000000000..489510244 --- /dev/null +++ b/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 diff --git a/test/libsolidity/semanticTests/array/push_no_args_bytes.sol b/test/libsolidity/semanticTests/array/push_no_args_bytes.sol new file mode 100644 index 000000000..dc99dbbd5 --- /dev/null +++ b/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 diff --git a/test/libsolidity/semanticTests/array/push_no_args_struct.sol b/test/libsolidity/semanticTests/array/push_no_args_struct.sol new file mode 100644 index 000000000..6f626d9c3 --- /dev/null +++ b/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