Memory structs.

libsolidity/CompilerUtils.cpp

@@ -436,19 +436,54 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp
 	}
 	case Type::Category::Struct:
 	{
-		//@todo we can probably use some of the code for arrays here.
 		solAssert(targetTypeCategory == stackTypeCategory, "");
 		auto& targetType = dynamic_cast<StructType const&>(_targetType);
-		auto& stackType = dynamic_cast<StructType const&>(_typeOnStack);
+		auto& typeOnStack = dynamic_cast<StructType const&>(_typeOnStack);
 		solAssert(
-			targetType.location() == DataLocation::Storage &&
-				stackType.location() == DataLocation::Storage,
-			"Non-storage structs not yet implemented."
-		);
-		solAssert(
-			targetType.isPointer(),
-			"Type conversion to non-pointer struct requested."
-		);
+			targetType.location() != DataLocation::CallData &&
+			typeOnStack.location() != DataLocation::CallData
+		, "");
+		switch (targetType.location())
+		{
+		case DataLocation::Storage:
+			// Other cases are done explicitly in LValue::storeValue, and only possible by assignment.
+			solAssert(
+				targetType.isPointer() &&
+				typeOnStack.location() == DataLocation::Storage,
+				"Invalid conversion to storage type."
+			);
+			break;
+		case DataLocation::Memory:
+			// Copy the array to a free position in memory, unless it is already in memory.
+			if (typeOnStack.location() != DataLocation::Memory)
+			{
+				solAssert(typeOnStack.location() == DataLocation::Storage, "");
+				// stack: <source ref> <source byte offset>
+				m_context << eth::Instruction::POP;
+				m_context << typeOnStack.memorySize();
+				allocateMemory();
+				m_context << eth::Instruction::SWAP1 << eth::Instruction::DUP2;
+				// stack: <memory ptr> <source ref> <memory ptr>
+				for (auto const& member: typeOnStack.getMembers())
+				{
+					if (!member.type->canLiveOutsideStorage())
+						continue;
+					pair<u256, unsigned> const& offsets = typeOnStack.getStorageOffsetsOfMember(member.name);
+					m_context << offsets.first << eth::Instruction::DUP3 << eth::Instruction::ADD;
+					m_context << u256(offsets.second);
+					StorageItem(m_context, *member.type).retrieveValue(SourceLocation(), true);
+					TypePointer targetMemberType = targetType.getMemberType(member.name);
+					solAssert(!!targetMemberType, "Member not found in target type.");
+					convertType(*member.type, *targetMemberType, true);
+					storeInMemoryDynamic(*targetMemberType, true);
+				}
+				m_context << eth::Instruction::POP << eth::Instruction::POP;
+			}
+			break;
+		case DataLocation::CallData:
+			solAssert(false, "");
+			break;
+		}
 		break;
 	}
 	default:

libsolidity/ExpressionCompiler.cpp

@@ -155,8 +155,6 @@ bool ExpressionCompiler::visit(Assignment const& _assignment)
 	TypePointer type = _assignment.getRightHandSide().getType();
 	if (!_assignment.getType()->dataStoredIn(DataLocation::Storage))
 	{
-		//@todo we should delay conversion here if RHS is not in memory, LHS is a MemoryItem
-		// and not dynamically-sized.
 		utils().convertType(*type, *_assignment.getType());
 		type = _assignment.getType();
 	}
@@ -691,7 +689,8 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess)
 		}
 		case DataLocation::Memory:
 		{
-			solAssert(false, "Member access for memory structs not yet implemented.");
+			m_context << type.memoryOffsetOfMember(member) << eth::Instruction::ADD;
+			setLValue<MemoryItem>(_memberAccess, *_memberAccess.getType());
 			break;
 		}
 		default:

libsolidity/LValue.cpp

@@ -47,6 +47,7 @@ void StackVariable::retrieveValue(SourceLocation const& _location, bool) const
 			errinfo_sourceLocation(_location) <<
 			errinfo_comment("Stack too deep, try removing local variables.")
 		);
+	solAssert(stackPos + 1 >= m_size, "Size and stack pos mismatch.");
 	for (unsigned i = 0; i < m_size; ++i)
 		m_context << eth::dupInstruction(stackPos + 1);
 }
@@ -175,6 +176,7 @@ void StorageItem::retrieveValue(SourceLocation const&, bool _remove) const
 
 void StorageItem::storeValue(Type const& _sourceType, SourceLocation const& _location, bool _move) const
 {
+	CompilerUtils utils(m_context);
 	// stack: value storage_key storage_offset
 	if (m_dataType.isValueType())
 	{
@@ -238,52 +240,66 @@ void StorageItem::storeValue(Type const& _sourceType, SourceLocation const& _loc
 						dynamic_cast<ArrayType const&>(m_dataType),
 						dynamic_cast<ArrayType const&>(_sourceType));
 			if (_move)
-				CompilerUtils(m_context).popStackElement(_sourceType);
+				utils.popStackElement(_sourceType);
 		}
 		else if (m_dataType.getCategory() == Type::Category::Struct)
 		{
-			// stack layout: source_ref source_offset target_ref target_offset
+			// stack layout: source_ref [source_offset] target_ref target_offset
 			// note that we have structs, so offsets should be zero and are ignored
 			auto const& structType = dynamic_cast<StructType const&>(m_dataType);
+			auto const& sourceType = dynamic_cast<StructType const&>(_sourceType);
 			solAssert(
-				structType.structDefinition() ==
-					dynamic_cast<StructType const&>(_sourceType).structDefinition(),
+				structType.structDefinition() == sourceType.structDefinition(),
 				"Struct assignment with conversion."
 			);
+			solAssert(sourceType.location() != DataLocation::CallData, "Structs in calldata not supported.");
 			for (auto const& member: structType.getMembers())
 			{
 				// assign each member that is not a mapping
 				TypePointer const& memberType = member.type;
 				if (memberType->getCategory() == Type::Category::Mapping)
 					continue;
+				TypePointer sourceMemberType = sourceType.getMemberType(member.name);
+				if (sourceType.location() == DataLocation::Storage)
+				{
+					// stack layout: source_ref source_offset target_ref target_offset
+					pair<u256, unsigned> const& offsets = sourceType.getStorageOffsetsOfMember(member.name);
+					m_context << offsets.first << eth::Instruction::DUP5 << eth::Instruction::ADD;
+					m_context << u256(offsets.second);
+					// stack: source_ref source_off target_ref target_off source_member_ref source_member_off
+					StorageItem(m_context, *sourceMemberType).retrieveValue(_location, true);
+					// stack: source_ref source_off target_ref target_off source_value...
+				}
+				else
+				{
+					solAssert(sourceType.location() == DataLocation::Memory, "");
+					// stack layout: source_ref target_ref target_offset
+					TypePointer sourceMemberType = sourceType.getMemberType(member.name);
+					m_context << sourceType.memoryOffsetOfMember(member.name);
+					m_context << eth::Instruction::DUP4 << eth::Instruction::ADD;
+					MemoryItem(m_context, *sourceMemberType).retrieveValue(_location, true);
+					// stack layout: source_ref target_ref target_offset source_value...
+				}
+				unsigned stackSize = sourceMemberType->getSizeOnStack();
 				pair<u256, unsigned> const& offsets = structType.getStorageOffsetsOfMember(member.name);
-				m_context
-					<< offsets.first << u256(offsets.second)
-					<< eth::Instruction::DUP6 << eth::Instruction::DUP3
-					<< eth::Instruction::ADD << eth::Instruction::DUP2;
-				// stack: source_ref source_off target_ref target_off member_slot_offset member_byte_offset source_member_ref source_member_off
-				StorageItem(m_context, *memberType).retrieveValue(_location, true);
-				// stack: source_ref source_off target_ref target_off member_offset source_value...
-				solAssert(
-					4 + memberType->getSizeOnStack() <= 16,
-					"Stack too deep, try removing local varibales."
-				);
-				m_context
-					<< eth::dupInstruction(4 + memberType->getSizeOnStack())
-					<< eth::dupInstruction(3 + memberType->getSizeOnStack()) << eth::Instruction::ADD
-					<< eth::dupInstruction(2 + memberType->getSizeOnStack());
-				// stack: source_ref source_off target_ref target_off member_slot_offset member_byte_offset source_value... target_member_ref target_member_byte_off
-				StorageItem(m_context, *memberType).storeValue(*memberType, _location, true);
-				m_context << eth::Instruction::POP << eth::Instruction::POP;
+				m_context << eth::dupInstruction(2 + stackSize) << offsets.first << eth::Instruction::ADD;
+				m_context << u256(offsets.second);
+				// stack: source_ref [source_off] target_ref target_off source_value... target_member_ref target_member_byte_off
+				StorageItem(m_context, *memberType).storeValue(*sourceMemberType, _location, true);
 			}
+			// stack layout: source_ref [source_offset] target_ref target_offset
+			unsigned sourceStackSize = sourceType.getSizeOnStack();
 			if (_move)
-				m_context
-					<< eth::Instruction::POP << eth::Instruction::POP
-					<< eth::Instruction::POP << eth::Instruction::POP;
-			else
-				m_context
-					<< eth::Instruction::SWAP2 << eth::Instruction::POP
-					<< eth::Instruction::SWAP2 << eth::Instruction::POP;
+				utils.popStackSlots(2 + sourceType.getSizeOnStack());
+			else if (sourceType.getSizeOnStack() >= 1)
+			{
+				// remove the source ref
+				solAssert(sourceStackSize <= 2, "");
+				m_context << eth::swapInstruction(sourceStackSize);
+				if (sourceStackSize == 2)
+					m_context << eth::Instruction::POP;
+				m_context << eth::Instruction::SWAP2 << eth::Instruction::POP;
+			}
 		}
 		else
 			BOOST_THROW_EXCEPTION(

libsolidity/LValue.h

@@ -103,7 +103,7 @@ private:
 class MemoryItem: public LValue
 {
 public:
-	MemoryItem(CompilerContext& _compilerContext, Type const& _type, bool _padded);
+	MemoryItem(CompilerContext& _compilerContext, Type const& _type, bool _padded = true);
 	virtual unsigned sizeOnStack() const override { return 1; }
 	virtual void retrieveValue(SourceLocation const& _location, bool _remove = false) const override;
 	virtual void storeValue(

libsolidity/Types.cpp

@@ -999,6 +999,15 @@ unsigned StructType::getCalldataEncodedSize(bool _padded) const
 	return size;
 }
 
+u256 StructType::memorySize() const
+{
+	u256 size;
+	for (auto const& member: getMembers())
+		if (member.type->canLiveOutsideStorage())
+			size += member.type->memoryHeadSize();
+	return size;
+}
+
 u256 StructType::getStorageSize() const
 {
 	return max<u256>(1, getMembers().getStorageSize());
@@ -1012,6 +1021,17 @@ bool StructType::canLiveOutsideStorage() const
 	return true;
 }
 
+unsigned StructType::getSizeOnStack() const
+{
+	switch (location())
+	{
+	case DataLocation::Storage:
+		return 2; // slot and offset
+	default:
+		return 1;
+	}
+}
+
 string StructType::toString(bool _short) const
 {
 	string ret = "struct " + m_struct.getName();
@@ -1054,6 +1074,18 @@ pair<u256, unsigned> const& StructType::getStorageOffsetsOfMember(string const&
 	return *offsets;
 }
 
+u256 StructType::memoryOffsetOfMember(string const& _name) const
+{
+	u256 offset;
+	for (auto const& member: getMembers())
+		if (member.name == _name)
+			return offset;
+		else
+			offset += member.type->memoryHeadSize();
+	solAssert(false, "Member not found in struct.");
+	return 0;
+}
+
 TypePointer EnumType::unaryOperatorResult(Token::Value _operator) const
 {
 	return _operator == Token::Delete ? make_shared<VoidType>() : TypePointer();

libsolidity/Types.h

@@ -543,14 +543,14 @@ class StructType: public ReferenceType
 public:
 	virtual Category getCategory() const override { return Category::Struct; }
 	explicit StructType(StructDefinition const& _struct):
-		//@todo only storage until we have non-storage structs
 		ReferenceType(DataLocation::Storage), m_struct(_struct) {}
 	virtual bool isImplicitlyConvertibleTo(const Type& _convertTo) const override;
 	virtual bool operator==(Type const& _other) const override;
 	virtual unsigned getCalldataEncodedSize(bool _padded) const override;
+	u256 memorySize() const;
 	virtual u256 getStorageSize() const override;
 	virtual bool canLiveOutsideStorage() const override;
-	virtual unsigned getSizeOnStack() const override { return 2; }
+	virtual unsigned getSizeOnStack() const override;
 	virtual std::string toString(bool _short) const override;
 
 	virtual MemberList const& getMembers() const override;
@@ -558,6 +558,7 @@ public:
 	TypePointer copyForLocation(DataLocation _location, bool _isPointer) const override;
 
 	std::pair<u256, unsigned> const& getStorageOffsetsOfMember(std::string const& _name) const;
+	u256 memoryOffsetOfMember(std::string const& _name) const;
 
 	StructDefinition const& structDefinition() const { return m_struct; }
 

test/libsolidity/SolidityEndToEndTest.cpp

@@ -4806,6 +4806,149 @@ BOOST_AUTO_TEST_CASE(memory_arrays_dynamic_index_access_write)
 	BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(0x20), u256(4), data));
 }
 
+BOOST_AUTO_TEST_CASE(memory_structs_read_write)
+{
+	char const* sourceCode = R"(
+		contract Test {
+			struct S { uint8 x; uint16 y; uint z; uint8[2] a; }
+			S[5] data;
+			function testInit() returns (uint8 x, uint16 y, uint z, uint8 a, bool flag) {
+				S[2] memory d;
+				x = d[0].x;
+				y = d[0].y;
+				z = d[0].z;
+				a = d[0].a[1];
+				flag = true;
+			}
+			function testCopyRead() returns (uint8 x, uint16 y, uint z, uint8 a) {
+				data[2].x = 1;
+				data[2].y = 2;
+				data[2].z = 3;
+				data[2].a[1] = 4;
+				S memory s = data[2];
+				x = s.x;
+				y = s.y;
+				z = s.z;
+				a = s.a[1];
+			}
+			function testAssign() returns (uint8 x, uint16 y, uint z, uint8 a) {
+				S memory s;
+				s.x = 1;
+				s.y = 2;
+				s.z = 3;
+				s.a[1] = 4;
+				x = s.x;
+				y = s.y;
+				z = s.z;
+				a = s.a[1];
+			}
+		}
+	)";
+	compileAndRun(sourceCode, 0, "Test");
+
+	BOOST_CHECK(callContractFunction("testInit()") == encodeArgs(u256(0), u256(0), u256(0), u256(0), true));
+	BOOST_CHECK(callContractFunction("testCopyRead()") == encodeArgs(u256(1), u256(2), u256(3), u256(4)));
+	BOOST_CHECK(callContractFunction("testAssign()") == encodeArgs(u256(1), u256(2), u256(3), u256(4)));
+}
+
+BOOST_AUTO_TEST_CASE(memory_structs_as_function_args)
+{
+	char const* sourceCode = R"(
+		contract Test {
+			struct S { uint8 x; uint16 y; uint z; }
+			function test() returns (uint x, uint y, uint z) {
+				S memory data = combine(1, 2, 3);
+				x = extract(data, 0);
+				y = extract(data, 1);
+				z = extract(data, 2);
+			}
+			function extract(S s, uint which) internal returns (uint x) {
+				if (which == 0) return s.x;
+				else if (which == 1) return s.y;
+				else return s.z;
+			}
+			function combine(uint8 x, uint16 y, uint z) internal returns (S s) {
+				s.x = x;
+				s.y = y;
+				s.z = z;
+			}
+		}
+	)";
+	compileAndRun(sourceCode, 0, "Test");
+
+	BOOST_CHECK(callContractFunction("test()") == encodeArgs(u256(1), u256(2), u256(3)));
+}
+
+BOOST_AUTO_TEST_CASE(memory_structs_wrapped)
+{
+	char const* sourceCode = R"(
+		contract Test {
+			struct S { uint8 x; uint16 y; uint z; }
+			struct X { uint8 x; S s; }
+			function test() returns (uint a, uint x, uint y, uint z) {
+				X memory d = combine(1, 2, 3, 4);
+				a = extract(d, 0);
+				x = extract(d, 1);
+				y = extract(d, 2);
+				z = extract(d, 3);
+			}
+			function extract(X s, uint which) internal returns (uint x) {
+				if (which == 0) return s.x;
+				else if (which == 1) return s.s.x;
+				else if (which == 2) return s.s.y;
+				else return s.s.z;
+			}
+			function combine(uint8 a, uint8 x, uint16 y, uint z) internal returns (X s) {
+				s.x = a;
+				s.s.x = x;
+				s.s.y = y;
+				s.s.z = z;
+			}
+		}
+	)";
+	compileAndRun(sourceCode, 0, "Test");
+
+	BOOST_CHECK(callContractFunction("test()") == encodeArgs(u256(1), u256(2), u256(3), u256(4)));
+}
+
+BOOST_AUTO_TEST_CASE(memory_structs_wrapped_load)
+{
+	char const* sourceCode = R"(
+		contract Test {
+			struct S { uint8 x; uint16 y; uint z; }
+			struct X { uint8 x; S s; }
+			X m_x;
+			function load() returns (uint a, uint x, uint y, uint z) {
+				m_x.x = 1;
+				m_x.s.x = 2;
+				m_x.s.y = 3;
+				m_x.s.z = 4;
+				X memory d = m_x;
+				a = d.x;
+				x = d.s.x;
+				y = d.s.y;
+				z = d.s.z;
+			}
+			function store() returns (uint a, uint x, uint y, uint z) {
+				X memory d = m_x;
+				d.x = 1;
+				d.s.x = 2;
+				d.s.y = 3;
+				d.s.z = 4;
+				m_x = d;
+				a = m_x.x;
+				x = m_x.s.x;
+				y = m_x.s.y;
+				z = m_x.s.z;
+			}
+		}
+	)";
+	compileAndRun(sourceCode, 0, "Test");
+
+	BOOST_CHECK(callContractFunction("load()") == encodeArgs(u256(1), u256(2), u256(3), u256(4)));
+	BOOST_CHECK(callContractFunction("store()") == encodeArgs(u256(1), u256(2), u256(3), u256(4)));
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }