Accessors for strings.

9 files changed, 182 insertions, 125 deletions

diff --git a/AST.cpp b/AST.cpp
index dbeec858..6d6a13cf 100644
--- a/AST.cpp
+++ b/AST.cpp
@@ -919,7 +919,7 @@ void MemberAccess::checkTypeRequirements(TypePointers const* _argumentTypes)
 	{
 		auto const& arrayType(dynamic_cast<ArrayType const&>(type));
 		m_isLValue = (*m_memberName == "length" &&
-			arrayType.location() != ReferenceType::Location::CallData && arrayType.isDynamicallySized());
+			arrayType.location() != DataLocation::CallData && arrayType.isDynamicallySized());
 	}
 	else
 		m_isLValue = false;
@@ -942,7 +942,7 @@ void IndexAccess::checkTypeRequirements(TypePointers const*)
 			m_type = make_shared<FixedBytesType>(1);
 		else
 			m_type = type.getBaseType();
-		m_isLValue = type.location() != ReferenceType::Location::CallData;
+		m_isLValue = type.location() != DataLocation::CallData;
 		break;
 	}
 	case Type::Category::Mapping:
@@ -959,7 +959,7 @@ void IndexAccess::checkTypeRequirements(TypePointers const*)
 	{
 		TypeType const& type = dynamic_cast<TypeType const&>(*m_base->getType());
 		if (!m_index)
-			m_type = make_shared<TypeType>(make_shared<ArrayType>(ReferenceType::Location::Memory, type.getActualType()));
+			m_type = make_shared<TypeType>(make_shared<ArrayType>(DataLocation::Memory, type.getActualType()));
 		else
 		{
 			m_index->checkTypeRequirements(nullptr);
@@ -967,7 +967,9 @@ void IndexAccess::checkTypeRequirements(TypePointers const*)
 			if (!length)
 				BOOST_THROW_EXCEPTION(m_index->createTypeError("Integer constant expected."));
 			m_type = make_shared<TypeType>(make_shared<ArrayType>(
-				ReferenceType::Location::Memory, type.getActualType(), length->literalValue(nullptr)));
+				DataLocation::Memory, type.getActualType(),
+				length->literalValue(nullptr)
+			));
 		}
 		break;
 	}
diff --git a/ArrayUtils.cpp b/ArrayUtils.cpp
index b770b815..43ffff0b 100644
--- a/ArrayUtils.cpp
+++ b/ArrayUtils.cpp
@@ -38,10 +38,10 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons
 	// need to leave "target_ref target_byte_off" on the stack at the end
 
 	// stack layout: [source_ref] [source_byte_off] [source length] target_ref target_byte_off (top)
-	solAssert(_targetType.location() == ReferenceType::Location::Storage, "");
+	solAssert(_targetType.location() == DataLocation::Storage, "");
 	solAssert(
-		_sourceType.location() == ReferenceType::Location::CallData ||
-			_sourceType.location() == ReferenceType::Location::Storage,
+		_sourceType.location() == DataLocation::CallData ||
+			_sourceType.location() == DataLocation::Storage,
 		"Given array location not implemented."
 	);
 
@@ -51,7 +51,7 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons
 
 	// TODO unroll loop for small sizes
 
-	bool sourceIsStorage = _sourceType.location() == ReferenceType::Location::Storage;
+	bool sourceIsStorage = _sourceType.location() == DataLocation::Storage;
 	bool directCopy = sourceIsStorage && sourceBaseType->isValueType() && *sourceBaseType == *targetBaseType;
 	bool haveByteOffsetSource = !directCopy && sourceIsStorage && sourceBaseType->getStorageBytes() <= 16;
 	bool haveByteOffsetTarget = !directCopy && targetBaseType->getStorageBytes() <= 16;
@@ -69,7 +69,7 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons
 		m_context << eth::Instruction::POP;
 	// stack: target_ref source_ref [source_length]
 	// retrieve source length
-	if (_sourceType.location() != ReferenceType::Location::CallData || !_sourceType.isDynamicallySized())
+	if (_sourceType.location() != DataLocation::CallData || !_sourceType.isDynamicallySized())
 		retrieveLength(_sourceType); // otherwise, length is already there
 	// stack: target_ref source_ref source_length
 	m_context << eth::Instruction::DUP3;
@@ -82,7 +82,7 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons
 	if (sourceBaseType->getCategory() == Type::Category::Mapping)
 	{
 		solAssert(targetBaseType->getCategory() == Type::Category::Mapping, "");
-		solAssert(_sourceType.location() == ReferenceType::Location::Storage, "");
+		solAssert(_sourceType.location() == DataLocation::Storage, "");
 		// nothing to copy
 		m_context
 			<< eth::Instruction::POP << eth::Instruction::POP
@@ -106,7 +106,7 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons
 	eth::AssemblyItem copyLoopEndWithoutByteOffset = m_context.newTag();
 	m_context.appendConditionalJumpTo(copyLoopEndWithoutByteOffset);
 
-	if (_sourceType.location() == ReferenceType::Location::Storage && _sourceType.isDynamicallySized())
+	if (_sourceType.location() == DataLocation::Storage && _sourceType.isDynamicallySized())
 		CompilerUtils(m_context).computeHashStatic();
 	// stack: target_ref target_data_end source_length target_data_pos source_data_pos
 	m_context << eth::Instruction::SWAP2;
@@ -155,7 +155,7 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons
 		// checking is easier.
 		// stack: target_ref target_data_end source_data_pos target_data_pos source_data_end [target_byte_offset] [source_byte_offset]
 		m_context << eth::dupInstruction(3 + byteOffsetSize);
-		if (_sourceType.location() == ReferenceType::Location::Storage)
+		if (_sourceType.location() == DataLocation::Storage)
 		{
 			if (haveByteOffsetSource)
 				m_context << eth::Instruction::DUP2;
@@ -231,7 +231,7 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons
 void ArrayUtils::clearArray(ArrayType const& _type) const
 {
 	unsigned stackHeightStart = m_context.getStackHeight();
-	solAssert(_type.location() == ReferenceType::Location::Storage, "");
+	solAssert(_type.location() == DataLocation::Storage, "");
 	if (_type.getBaseType()->getStorageBytes() < 32)
 	{
 		solAssert(_type.getBaseType()->isValueType(), "Invalid storage size for non-value type.");
@@ -286,7 +286,7 @@ void ArrayUtils::clearArray(ArrayType const& _type) const
 
 void ArrayUtils::clearDynamicArray(ArrayType const& _type) const
 {
-	solAssert(_type.location() == ReferenceType::Location::Storage, "");
+	solAssert(_type.location() == DataLocation::Storage, "");
 	solAssert(_type.isDynamicallySized(), "");
 
 	unsigned stackHeightStart = m_context.getStackHeight();
@@ -314,7 +314,7 @@ void ArrayUtils::clearDynamicArray(ArrayType const& _type) const
 
 void ArrayUtils::resizeDynamicArray(const ArrayType& _type) const
 {
-	solAssert(_type.location() == ReferenceType::Location::Storage, "");
+	solAssert(_type.location() == DataLocation::Storage, "");
 	solAssert(_type.isDynamicallySized(), "");
 	if (!_type.isByteArray() && _type.getBaseType()->getStorageBytes() < 32)
 		solAssert(_type.getBaseType()->isValueType(), "Invalid storage size for non-value type.");
@@ -399,7 +399,7 @@ void ArrayUtils::clearStorageLoop(Type const& _type) const
 
 void ArrayUtils::convertLengthToSize(ArrayType const& _arrayType, bool _pad) const
 {
-	if (_arrayType.location() == ReferenceType::Location::Storage)
+	if (_arrayType.location() == DataLocation::Storage)
 	{
 		if (_arrayType.getBaseType()->getStorageSize() <= 1)
 		{
@@ -437,13 +437,13 @@ void ArrayUtils::retrieveLength(ArrayType const& _arrayType) const
 		m_context << eth::Instruction::DUP1;
 		switch (_arrayType.location())
 		{
-		case ReferenceType::Location::CallData:
+		case DataLocation::CallData:
 			// length is stored on the stack
 			break;
-		case ReferenceType::Location::Memory:
+		case DataLocation::Memory:
 			m_context << eth::Instruction::MLOAD;
 			break;
-		case ReferenceType::Location::Storage:
+		case DataLocation::Storage:
 			m_context << eth::Instruction::SLOAD;
 			break;
 		}
@@ -452,16 +452,16 @@ void ArrayUtils::retrieveLength(ArrayType const& _arrayType) const
 
 void ArrayUtils::accessIndex(ArrayType const& _arrayType) const
 {
-	ReferenceType::Location location = _arrayType.location();
+	DataLocation location = _arrayType.location();
 	eth::Instruction load =
-		location == ReferenceType::Location::Storage ? eth::Instruction::SLOAD :
-		location == ReferenceType::Location::Memory ? eth::Instruction::MLOAD :
+		location == DataLocation::Storage ? eth::Instruction::SLOAD :
+		location == DataLocation::Memory ? eth::Instruction::MLOAD :
 		eth::Instruction::CALLDATALOAD;
 
 	// retrieve length
 	if (!_arrayType.isDynamicallySized())
 		m_context << _arrayType.getLength();
-	else if (location == ReferenceType::Location::CallData)
+	else if (location == DataLocation::CallData)
 		// length is stored on the stack
 		m_context << eth::Instruction::SWAP1;
 	else
@@ -476,20 +476,20 @@ void ArrayUtils::accessIndex(ArrayType const& _arrayType) const
 	m_context << eth::Instruction::SWAP1;
 	if (_arrayType.isDynamicallySized())
 	{
-		if (location == ReferenceType::Location::Storage)
+		if (location == DataLocation::Storage)
 			CompilerUtils(m_context).computeHashStatic();
-		else if (location == ReferenceType::Location::Memory)
+		else if (location == DataLocation::Memory)
 			m_context << u256(32) << eth::Instruction::ADD;
 	}
 	// stack: <index> <data_ref>
 	switch (location)
 	{
-	case ReferenceType::Location::CallData:
+	case DataLocation::CallData:
 		if (!_arrayType.isByteArray())
-			m_context
-				<< eth::Instruction::SWAP1
-				<< _arrayType.getBaseType()->getCalldataEncodedSize()
-				<< eth::Instruction::MUL;
+		{
+			m_context << eth::Instruction::SWAP1;
+			m_context << _arrayType.getBaseType()->getCalldataEncodedSize() << eth::Instruction::MUL;
+		}
 		m_context << eth::Instruction::ADD;
 		if (_arrayType.getBaseType()->isValueType())
 			CompilerUtils(m_context).loadFromMemoryDynamic(
@@ -499,7 +499,7 @@ void ArrayUtils::accessIndex(ArrayType const& _arrayType) const
 				false
 			);
 		break;
-	case ReferenceType::Location::Storage:
+	case DataLocation::Storage:
 		m_context << eth::Instruction::SWAP1;
 		if (_arrayType.getBaseType()->getStorageBytes() <= 16)
 		{
@@ -527,7 +527,7 @@ void ArrayUtils::accessIndex(ArrayType const& _arrayType) const
 			m_context << eth::Instruction::ADD << u256(0);
 		}
 		break;
-	case ReferenceType::Location::Memory:
+	case DataLocation::Memory:
 		solAssert(false, "Memory lvalues not yet implemented.");
 	}
 }
diff --git a/Compiler.cpp b/Compiler.cpp
index 82e98dff..0b88ed8a 100644
--- a/Compiler.cpp
+++ b/Compiler.cpp
@@ -245,21 +245,35 @@ void Compiler::appendCalldataUnpacker(
 {
 	// We do not check the calldata size, everything is zero-paddedd
 
+	//@todo this does not yet support nested arrays
+
 	if (_startOffset == u256(-1))
 		_startOffset = u256(CompilerUtils::dataStartOffset);
 
 	m_context << _startOffset;
 	for (TypePointer const& type: _typeParameters)
 	{
+		// stack: v1 v2 ... v(k-1) mem_offset
 		switch (type->getCategory())
 		{
 		case Type::Category::Array:
 		{
 			auto const& arrayType = dynamic_cast<ArrayType const&>(*type);
-			if (arrayType.location() == ReferenceType::Location::CallData)
+			solAssert(arrayType.location() != DataLocation::Storage, "");
+			solAssert(!arrayType.getBaseType()->isDynamicallySized(), "Nested arrays not yet implemented.");
+			if (_fromMemory)
+			{
+				solAssert(arrayType.location() == DataLocation::Memory, "");
+				// compute data pointer
+				//@todo once we support nested arrays, this offset needs to be dynamic.
+				m_context << eth::Instruction::DUP1 << _startOffset << eth::Instruction::ADD;
+				m_context << eth::Instruction::SWAP1 << u256(0x20) << eth::Instruction::ADD;
+			}
+			else
 			{
-				solAssert(!_fromMemory, "");
-				if (type->isDynamicallySized())
+				// first load from calldata and potentially convert to memory if arrayType is memory
+				TypePointer calldataType = arrayType.copyForLocation(DataLocation::CallData, false);
+				if (calldataType->isDynamicallySized())
 				{
 					// put on stack: data_pointer length
 					CompilerUtils(m_context).loadFromMemoryDynamic(IntegerType(256), !_fromMemory);
@@ -276,17 +290,17 @@ void Compiler::appendCalldataUnpacker(
 				{
 					// leave the pointer on the stack
 					m_context << eth::Instruction::DUP1;
-					m_context << u256(type->getCalldataEncodedSize()) << eth::Instruction::ADD;
+					m_context << u256(calldataType->getCalldataEncodedSize()) << eth::Instruction::ADD;
+				}
+				if (arrayType.location() == DataLocation::Memory)
+				{
+					// copy to memory
+					// move calldata type up again
+					CompilerUtils(m_context).moveIntoStack(calldataType->getSizeOnStack());
+					CompilerUtils(m_context).convertType(*calldataType, arrayType);
+					// fetch next pointer again
+					CompilerUtils(m_context).moveToStackTop(arrayType.getSizeOnStack());
 				}
-			}
-			else
-			{
-				solAssert(arrayType.location() == ReferenceType::Location::Memory, "");
-				// compute data pointer
-				m_context << eth::Instruction::DUP1 << _startOffset << eth::Instruction::ADD;
-				if (!_fromMemory)
-					solAssert(false, "Not yet implemented.");
-				m_context << eth::Instruction::SWAP1 << u256(0x20) << eth::Instruction::ADD;
 			}
 			break;
 		}
diff --git a/CompilerUtils.cpp b/CompilerUtils.cpp
index d4e705a3..4d57dc92 100644
--- a/CompilerUtils.cpp
+++ b/CompilerUtils.cpp
@@ -107,16 +107,18 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound
 		auto const& type = dynamic_cast<ArrayType const&>(_type);
 		solAssert(type.isByteArray(), "Non byte arrays not yet implemented here.");
 
-		if (type.location() == ReferenceType::Location::CallData)
+		if (type.location() == DataLocation::CallData)
 		{
+			if (!type.isDynamicallySized())
+				m_context << type.getLength();
 			// stack: target source_offset source_len
 			m_context << eth::Instruction::DUP1 << eth::Instruction::DUP3 << eth::Instruction::DUP5;
-				// stack: target source_offset source_len source_len source_offset target
+			// stack: target source_offset source_len source_len source_offset target
 			m_context << eth::Instruction::CALLDATACOPY;
 			m_context << eth::Instruction::DUP3 << eth::Instruction::ADD;
 			m_context << eth::Instruction::SWAP2 << eth::Instruction::POP << eth::Instruction::POP;
 		}
-		else if (type.location() == ReferenceType::Location::Memory)
+		else if (type.location() == DataLocation::Memory)
 		{
 			// memcpy using the built-in contract
 			ArrayUtils(m_context).retrieveLength(type);
@@ -183,7 +185,7 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound
 		}
 		else
 		{
-			solAssert(type.location() == ReferenceType::Location::Storage, "");
+			solAssert(type.location() == DataLocation::Storage, "");
 			m_context << eth::Instruction::POP; // remove offset, arrays always start new slot
 			m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD;
 			// stack here: memory_offset storage_offset length_bytes
@@ -276,7 +278,10 @@ void CompilerUtils::encodeToMemory(
 			copyToStackTop(argSize - stackPos + dynPointers + 2, _givenTypes[i]->getSizeOnStack());
 			solAssert(!!targetType, "Externalable type expected.");
 			TypePointer type = targetType;
-			if (_givenTypes[i]->isInStorage())
+			if (
+				_givenTypes[i]->dataStoredIn(DataLocation::Storage) ||
+				_givenTypes[i]->dataStoredIn(DataLocation::CallData)
+			)
 				type = _givenTypes[i]; // delay conversion
 			else
 				convertType(*_givenTypes[i], *targetType, true);
@@ -307,13 +312,13 @@ void CompilerUtils::encodeToMemory(
 			// stack: ... <end_of_mem> <value...>
 			// copy length to memory
 			m_context << eth::dupInstruction(1 + arrayType.getSizeOnStack());
-			if (arrayType.location() == ReferenceType::Location::CallData)
+			if (arrayType.location() == DataLocation::CallData)
 				m_context << eth::Instruction::DUP2; // length is on stack
-			else if (arrayType.location() == ReferenceType::Location::Storage)
+			else if (arrayType.location() == DataLocation::Storage)
 				m_context << eth::Instruction::DUP3 << eth::Instruction::SLOAD;
 			else
 			{
-				solAssert(arrayType.location() == ReferenceType::Location::Memory, "");
+				solAssert(arrayType.location() == DataLocation::Memory, "");
 				m_context << eth::Instruction::DUP2 << eth::Instruction::MLOAD;
 			}
 			// stack: ... <end_of_mem> <value...> <end_of_mem'> <length>
@@ -435,18 +440,18 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp
 		ArrayType const& targetType = dynamic_cast<ArrayType const&>(_targetType);
 		switch (targetType.location())
 		{
-		case ReferenceType::Location::Storage:
+		case DataLocation::Storage:
 			// Other cases are done explicitly in LValue::storeValue, and only possible by assignment.
 			solAssert(
 				targetType.isPointer() &&
-				typeOnStack.location() == ReferenceType::Location::Storage,
+				typeOnStack.location() == DataLocation::Storage,
 				"Invalid conversion to storage type."
 			);
 			break;
-		case ReferenceType::Location::Memory:
+		case DataLocation::Memory:
 		{
 			// Copy the array to a free position in memory, unless it is already in memory.
-			if (typeOnStack.location() != ReferenceType::Location::Memory)
+			if (typeOnStack.location() != DataLocation::Memory)
 			{
 				// stack: <source ref> (variably sized)
 				unsigned stackSize = typeOnStack.getSizeOnStack();
@@ -455,7 +460,7 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp
 				// stack: <mem start> <source ref> (variably sized)
 				if (targetType.isDynamicallySized())
 				{
-					bool fromStorage = (typeOnStack.location() == ReferenceType::Location::Storage);
+					bool fromStorage = (typeOnStack.location() == DataLocation::Storage);
 					// store length
 					if (fromStorage)
 					{
@@ -486,11 +491,25 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp
 				// Stack <mem start> <mem end>
 				storeFreeMemoryPointer();
 			}
-			else if (typeOnStack.location() == ReferenceType::Location::CallData)
+			else if (typeOnStack.location() == DataLocation::CallData)
 			{
-				// Stack: <offset> <length>
-				//@todo
-				solAssert(false, "Not yet implemented.");
+				// Stack: <offset> [<length>]
+				// length is present if dynamically sized
+				fetchFreeMemoryPointer();
+				moveIntoStack(typeOnStack.getSizeOnStack());
+				// stack: memptr calldataoffset [<length>]
+				if (typeOnStack.isDynamicallySized())
+				{
+					solAssert(targetType.isDynamicallySized(), "");
+					m_context << eth::Instruction::DUP3 << eth::Instruction::DUP2;
+					storeInMemoryDynamic(IntegerType(256));
+					moveIntoStack(typeOnStack.getSizeOnStack());
+				}
+				else
+					m_context << eth::Instruction::DUP2 << eth::Instruction::SWAP1;
+				// stack: mem_ptr mem_data_ptr calldataoffset [<length>]
+				storeInMemoryDynamic(typeOnStack);
+				storeFreeMemoryPointer();
 			}
 			// nothing to do for memory to memory
 			break;
@@ -507,8 +526,8 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp
 		auto& targetType = dynamic_cast<StructType const&>(_targetType);
 		auto& stackType = dynamic_cast<StructType const&>(_typeOnStack);
 		solAssert(
-			targetType.location() == ReferenceType::Location::Storage &&
-				stackType.location() == ReferenceType::Location::Storage,
+			targetType.location() == DataLocation::Storage &&
+				stackType.location() == DataLocation::Storage,
 			"Non-storage structs not yet implemented."
 		);
 		solAssert(
diff --git a/CompilerUtils.h b/CompilerUtils.h
index 32dc93a2..a880f9ee 100644
--- a/CompilerUtils.h
+++ b/CompilerUtils.h
@@ -99,8 +99,9 @@ public:
 		bool _copyDynamicDataInPlace = false
 	);
 
-	/// Appends code for an implicit or explicit type conversion. For now this comprises only erasing
-	/// higher-order bits (@see appendHighBitCleanup) when widening integer.
+	/// Appends code for an implicit or explicit type conversion. This includes erasing higher
+	/// order bits (@see appendHighBitCleanup) when widening integer but also copy to memory
+	/// if a reference type is converted from calldata or storage to memory.
 	/// If @a _cleanupNeeded, high order bits cleanup is also done if no type conversion would be
 	/// necessary.
 	void convertType(Type const& _typeOnStack, Type const& _targetType, bool _cleanupNeeded = false);
diff --git a/ExpressionCompiler.cpp b/ExpressionCompiler.cpp
index c98d76f3..bfb945d8 100644
--- a/ExpressionCompiler.cpp
+++ b/ExpressionCompiler.cpp
@@ -109,34 +109,40 @@ void ExpressionCompiler::appendStateVariableAccessor(VariableDeclaration const&
 	}
 	unsigned retSizeOnStack = 0;
 	solAssert(accessorType.getReturnParameterTypes().size() >= 1, "");
+	auto const& returnTypes = accessorType.getReturnParameterTypes();
 	if (StructType const* structType = dynamic_cast<StructType const*>(returnType.get()))
 	{
 		// remove offset
 		m_context << eth::Instruction::POP;
 		auto const& names = accessorType.getReturnParameterNames();
-		auto const& types = accessorType.getReturnParameterTypes();
 		// struct
 		for (size_t i = 0; i < names.size(); ++i)
 		{
-			if (types[i]->getCategory() == Type::Category::Mapping || types[i]->getCategory() == Type::Category::Array)
+			if (returnTypes[i]->getCategory() == Type::Category::Mapping)
 				continue;
+			if (auto arrayType = dynamic_cast<ArrayType const*>(returnTypes[i].get()))
+				if (!arrayType->isByteArray())
+					continue;
 			pair<u256, unsigned> const& offsets = structType->getStorageOffsetsOfMember(names[i]);
 			m_context << eth::Instruction::DUP1 << u256(offsets.first) << eth::Instruction::ADD << u256(offsets.second);
-			StorageItem(m_context, *types[i]).retrieveValue(SourceLocation(), true);
-			solAssert(types[i]->getSizeOnStack() == 1, "Returning struct elements with stack size != 1 is not yet implemented.");
-			m_context << eth::Instruction::SWAP1;
-			retSizeOnStack += types[i]->getSizeOnStack();
+			TypePointer memberType = structType->getMemberType(names[i]);
+			StorageItem(m_context, *memberType).retrieveValue(SourceLocation(), true);
+			utils().convertType(*memberType, *returnTypes[i]);
+			utils().moveToStackTop(returnTypes[i]->getSizeOnStack());
+			retSizeOnStack += returnTypes[i]->getSizeOnStack();
 		}
 		// remove slot
 		m_context << eth::Instruction::POP;
 	}
 	else
 	{
-		// simple value
-		solAssert(accessorType.getReturnParameterTypes().size() == 1, "");
+		// simple value or array
+		solAssert(returnTypes.size() == 1, "");
 		StorageItem(m_context, *returnType).retrieveValue(SourceLocation(), true);
-		retSizeOnStack = returnType->getSizeOnStack();
+		utils().convertType(*returnType, *returnTypes.front());
+		retSizeOnStack = returnTypes.front()->getSizeOnStack();
 	}
+	solAssert(retSizeOnStack == utils().getSizeOnStack(returnTypes), "");
 	solAssert(retSizeOnStack <= 15, "Stack is too deep.");
 	m_context << eth::dupInstruction(retSizeOnStack + 1);
 	m_context.appendJump(eth::AssemblyItem::JumpType::OutOfFunction);
@@ -147,7 +153,7 @@ bool ExpressionCompiler::visit(Assignment const& _assignment)
 	CompilerContext::LocationSetter locationSetter(m_context, _assignment);
 	_assignment.getRightHandSide().accept(*this);
 	TypePointer type = _assignment.getRightHandSide().getType();
-	if (!_assignment.getType()->isInStorage())
+	if (!_assignment.getType()->dataStoredIn(DataLocation::Storage))
 	{
 		utils().convertType(*type, *_assignment.getType());
 		type = _assignment.getType();
@@ -712,10 +718,10 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess)
 		else
 			switch (type.location())
 			{
-			case ReferenceType::Location::CallData:
+			case DataLocation::CallData:
 				m_context << eth::Instruction::SWAP1 << eth::Instruction::POP;
 				break;
-			case ReferenceType::Location::Storage:
+			case DataLocation::Storage:
 				setLValue<StorageArrayLength>(_memberAccess, type);
 				break;
 			default:
@@ -758,13 +764,13 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess)
 		solAssert(_indexAccess.getIndexExpression(), "Index expression expected.");
 
 		// remove storage byte offset
-		if (arrayType.location() == ReferenceType::Location::Storage)
+		if (arrayType.location() == DataLocation::Storage)
 			m_context << eth::Instruction::POP;
 
 		_indexAccess.getIndexExpression()->accept(*this);
 		// stack layout: <base_ref> [<length>] <index>
 		ArrayUtils(m_context).accessIndex(arrayType);
-		if (arrayType.location() == ReferenceType::Location::Storage)
+		if (arrayType.location() == DataLocation::Storage)
 		{
 			if (arrayType.isByteArray())
 			{
diff --git a/NameAndTypeResolver.cpp b/NameAndTypeResolver.cpp
index e6079796..87f9da7e 100644
--- a/NameAndTypeResolver.cpp
+++ b/NameAndTypeResolver.cpp
@@ -439,7 +439,7 @@ void ReferencesResolver::endVisit(VariableDeclaration& _variable)
 						"Location has to be calldata for external functions "
 						"(remove the \"memory\" or \"storage\" keyword)."
 					));
-				type = ref->copyForLocation(ReferenceType::Location::CallData, true);
+				type = ref->copyForLocation(DataLocation::CallData, true);
 			}
 			else if (_variable.isCallableParameter() && _variable.getScope()->isPublic())
 			{
@@ -449,7 +449,7 @@ void ReferencesResolver::endVisit(VariableDeclaration& _variable)
 						"Location has to be memory for publicly visible functions "
 						"(remove the \"storage\" keyword)."
 					));
-				type = ref->copyForLocation(ReferenceType::Location::Memory, true);
+				type = ref->copyForLocation(DataLocation::Memory, true);
 			}
 			else
 			{
@@ -458,8 +458,8 @@ void ReferencesResolver::endVisit(VariableDeclaration& _variable)
 				bool isPointer = !_variable.isStateVariable();
 				type = ref->copyForLocation(
 					loc == Location::Memory ?
-					ReferenceType::Location::Memory :
-					ReferenceType::Location::Storage,
+					DataLocation::Memory :
+					DataLocation::Storage,
 					isPointer
 				);
 			}
diff --git a/Types.cpp b/Types.cpp
index bedd2e7b..b90b8884 100644
--- a/Types.cpp
+++ b/Types.cpp
@@ -144,12 +144,13 @@ TypePointer Type::fromElementaryTypeName(Token::Value _typeToken)
 	else if (_typeToken == Token::Bool)
 		return make_shared<BoolType>();
 	else if (_typeToken == Token::Bytes)
-		return make_shared<ArrayType>(ReferenceType::Location::Storage);
+		return make_shared<ArrayType>(DataLocation::Storage);
 	else if (_typeToken == Token::String)
-		return make_shared<ArrayType>(ReferenceType::Location::Storage, true);
+		return make_shared<ArrayType>(DataLocation::Storage, true);
 	else
-		BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Unable to convert elementary typename " +
-																		 std::string(Token::toString(_typeToken)) + " to type."));
+		BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment(
+			"Unable to convert elementary typename " + std::string(Token::toString(_typeToken)) + " to type."
+		));
 }
 
 TypePointer Type::fromElementaryTypeName(string const& _name)
@@ -180,7 +181,7 @@ TypePointer Type::fromMapping(ElementaryTypeName& _keyType, TypeName& _valueType
 	if (!valueType)
 		BOOST_THROW_EXCEPTION(_valueType.createTypeError("Invalid type name."));
 	// Convert value type to storage reference.
-	valueType = ReferenceType::copyForLocationIfReference(ReferenceType::Location::Storage, valueType);
+	valueType = ReferenceType::copyForLocationIfReference(DataLocation::Storage, valueType);
 	return make_shared<MappingType>(keyType, valueType);
 }
 
@@ -198,10 +199,10 @@ TypePointer Type::fromArrayTypeName(TypeName& _baseTypeName, Expression* _length
 		auto const* length = dynamic_cast<IntegerConstantType const*>(_length->getType().get());
 		if (!length)
 			BOOST_THROW_EXCEPTION(_length->createTypeError("Invalid array length."));
-		return make_shared<ArrayType>(ReferenceType::Location::Storage, baseType, length->literalValue(nullptr));
+		return make_shared<ArrayType>(DataLocation::Storage, baseType, length->literalValue(nullptr));
 	}
 	else
-		return make_shared<ArrayType>(ReferenceType::Location::Storage, baseType);
+		return make_shared<ArrayType>(DataLocation::Storage, baseType);
 }
 
 TypePointer Type::forLiteral(Literal const& _literal)
@@ -670,7 +671,7 @@ TypePointer ContractType::unaryOperatorResult(Token::Value _operator) const
 	return _operator == Token::Delete ? make_shared<VoidType>() : TypePointer();
 }
 
-TypePointer ReferenceType::copyForLocationIfReference(Location _location, TypePointer const& _type)
+TypePointer ReferenceType::copyForLocationIfReference(DataLocation _location, TypePointer const& _type)
 {
 	if (auto type = dynamic_cast<ReferenceType const*>(_type.get()))
 		return type->copyForLocation(_location, false);
@@ -686,11 +687,11 @@ string ReferenceType::stringForReferencePart() const
 {
 	switch (m_location)
 	{
-	case Location::Storage:
+	case DataLocation::Storage:
 		return string("storage ") + (m_isPointer ? "pointer" : "ref");
-	case Location::CallData:
+	case DataLocation::CallData:
 		return "calldata";
-	case Location::Memory:
+	case DataLocation::Memory:
 		return "memory";
 	}
 	solAssert(false, "");
@@ -705,11 +706,11 @@ bool ArrayType::isImplicitlyConvertibleTo(const Type& _convertTo) const
 	if (convertTo.isByteArray() != isByteArray() || convertTo.isString() != isString())
 		return false;
 	// memory/calldata to storage can be converted, but only to a direct storage reference
-	if (convertTo.location() == Location::Storage && location() != Location::Storage && convertTo.isPointer())
+	if (convertTo.location() == DataLocation::Storage && location() != DataLocation::Storage && convertTo.isPointer())
 		return false;
-	if (convertTo.location() == Location::CallData && location() != convertTo.location())
+	if (convertTo.location() == DataLocation::CallData && location() != convertTo.location())
 		return false;
-	if (convertTo.location() == Location::Storage && !convertTo.isPointer())
+	if (convertTo.location() == DataLocation::Storage && !convertTo.isPointer())
 	{
 		// Less restrictive conversion, since we need to copy anyway.
 		if (!getBaseType()->isImplicitlyConvertibleTo(*convertTo.getBaseType()))
@@ -788,10 +789,10 @@ u256 ArrayType::getStorageSize() const
 
 unsigned ArrayType::getSizeOnStack() const
 {
-	if (m_location == Location::CallData)
+	if (m_location == DataLocation::CallData)
 		// offset [length] (stack top)
 		return 1 + (isDynamicallySized() ? 1 : 0);
-	else if (m_location == Location::Storage)
+	else if (m_location == DataLocation::Storage)
 		// storage_key storage_offset
 		return 2;
 	else
@@ -828,12 +829,12 @@ TypePointer ArrayType::externalType() const
 		return TypePointer();
 
 	if (isDynamicallySized())
-		return std::make_shared<ArrayType>(Location::CallData, m_baseType->externalType());
+		return std::make_shared<ArrayType>(DataLocation::CallData, m_baseType->externalType());
 	else
-		return std::make_shared<ArrayType>(Location::CallData, m_baseType->externalType(), m_length);
+		return std::make_shared<ArrayType>(DataLocation::CallData, m_baseType->externalType(), m_length);
 }
 
-TypePointer ArrayType::copyForLocation(ReferenceType::Location _location, bool _isPointer) const
+TypePointer ArrayType::copyForLocation(DataLocation _location, bool _isPointer) const
 {
 	auto copy = make_shared<ArrayType>(_location);
 	copy->m_isPointer = _isPointer;
@@ -949,9 +950,9 @@ bool StructType::isImplicitlyConvertibleTo(const Type& _convertTo) const
 		return false;
 	auto& convertTo = dynamic_cast<StructType const&>(_convertTo);
 	// memory/calldata to storage can be converted, but only to a direct storage reference
-	if (convertTo.location() == Location::Storage && location() != Location::Storage && convertTo.isPointer())
+	if (convertTo.location() == DataLocation::Storage && location() != DataLocation::Storage && convertTo.isPointer())
 		return false;
-	if (convertTo.location() == Location::CallData && location() != convertTo.location())
+	if (convertTo.location() == DataLocation::CallData && location() != convertTo.location())
 		return false;
 	return this->m_struct == convertTo.m_struct;
 }
@@ -1009,7 +1010,7 @@ MemberList const& StructType::getMembers() const
 	return *m_members;
 }
 
-TypePointer StructType::copyForLocation(ReferenceType::Location _location, bool _isPointer) const
+TypePointer StructType::copyForLocation(DataLocation _location, bool _isPointer) const
 {
 	auto copy = make_shared<StructType>(m_struct);
 	copy->m_location = _location;
@@ -1115,6 +1116,9 @@ FunctionType::FunctionType(VariableDeclaration const& _varDecl):
 		}
 		else if (auto arrayType = dynamic_cast<ArrayType const*>(returnType.get()))
 		{
+			if (arrayType->isByteArray())
+				// Return byte arrays as as whole.
+				break;
 			returnType = arrayType->getBaseType();
 			paramNames.push_back("");
 			paramTypes.push_back(make_shared<IntegerType>(256));
@@ -1128,15 +1132,21 @@ FunctionType::FunctionType(VariableDeclaration const& _varDecl):
 	if (auto structType = dynamic_cast<StructType const*>(returnType.get()))
 	{
 		for (auto const& member: structType->getMembers())
-			if (member.type->getCategory() != Category::Mapping && member.type->getCategory() != Category::Array)
+			if (member.type->getCategory() != Category::Mapping)
 			{
-				retParamNames.push_back(member.name);
+				if (auto arrayType = dynamic_cast<ArrayType const*>(member.type.get()))
+					if (!arrayType->isByteArray())
+						continue;
 				retParams.push_back(member.type);
+				retParamNames.push_back(member.name);
 			}
 	}
 	else
 	{
-		retParams.push_back(returnType);
+		retParams.push_back(ReferenceType::copyForLocationIfReference(
+			DataLocation::Memory,
+			returnType
+		));
 		retParamNames.push_back("");
 	}
 
@@ -1549,7 +1559,7 @@ MagicType::MagicType(MagicType::Kind _kind):
 			{"sender", make_shared<IntegerType>(0, IntegerType::Modifier::Address)},
 			{"gas", make_shared<IntegerType>(256)},
 			{"value", make_shared<IntegerType>(256)},
-			{"data", make_shared<ArrayType>(ReferenceType::Location::CallData)},
+			{"data", make_shared<ArrayType>(DataLocation::CallData)},
 			{"sig", make_shared<FixedBytesType>(4)}
 		});
 		break;
diff --git a/Types.h b/Types.h
index 70f7807c..cca5dde7 100644
--- a/Types.h
+++ b/Types.h
@@ -44,6 +44,8 @@ using FunctionTypePointer = std::shared_ptr<FunctionType const>;
 using TypePointers = std::vector<TypePointer>;
 
 
+enum class DataLocation { Storage, CallData, Memory };
+
 /**
  * Helper class to compute storage offsets of members of structs and contracts.
  */
@@ -202,8 +204,9 @@ public:
 	/// This returns the corresponding integer type for IntegerConstantTypes and the pointer type
 	/// for storage reference types.
 	virtual TypePointer mobileType() const { return shared_from_this(); }
-	/// @returns true if this type is a storage pointer or reference.
-	virtual bool isInStorage() const { return false; }
+	/// @returns true if this is a non-value type and the data of this type is stored at the
+	/// given location.
+	virtual bool dataStoredIn(DataLocation) const { return false; }
 
 	/// Returns the list of all members of this type. Default implementation: no members.
 	virtual MemberList const& getMembers() const { return EmptyMemberList; }
@@ -367,16 +370,15 @@ public:
 class ReferenceType: public Type
 {
 public:
-	enum class Location { Storage, CallData, Memory };
-	explicit ReferenceType(Location _location): m_location(_location) {}
-	Location location() const { return m_location; }
+	explicit ReferenceType(DataLocation _location): m_location(_location) {}
+	DataLocation location() const { return m_location; }
 
 	/// @returns a copy of this type with location (recursively) changed to @a _location,
 	/// whereas isPointer is only shallowly changed - the deep copy is always a bound reference.
-	virtual TypePointer copyForLocation(Location _location, bool _isPointer) const = 0;
+	virtual TypePointer copyForLocation(DataLocation _location, bool _isPointer) const = 0;
 
 	virtual TypePointer mobileType() const override { return copyForLocation(m_location, true); }
-	virtual bool isInStorage() const override { return m_location == Location::Storage; }
+	virtual bool dataStoredIn(DataLocation _location) const override { return m_location == _location; }
 
 	/// Storage references can be pointers or bound references. In general, local variables are of
 	/// pointer type, state variables are bound references. Assignments to pointers or deleting
@@ -392,14 +394,14 @@ public:
 	/// @returns a copy of @a _type having the same location as this (and is not a pointer type)
 	/// if _type is a reference type and an unmodified copy of _type otherwise.
 	/// This function is mostly useful to modify inner types appropriately.
-	static TypePointer copyForLocationIfReference(Location _location, TypePointer const& _type);
+	static TypePointer copyForLocationIfReference(DataLocation _location, TypePointer const& _type);
 
 protected:
 	TypePointer copyForLocationIfReference(TypePointer const& _type) const;
 	/// @returns a human-readable description of the reference part of the type.
 	std::string stringForReferencePart() const;
 
-	Location m_location = Location::Storage;
+	DataLocation m_location = DataLocation::Storage;
 	bool m_isPointer = true;
 };
 
@@ -416,20 +418,20 @@ public:
 	virtual Category getCategory() const override { return Category::Array; }
 
 	/// Constructor for a byte array ("bytes") and string.
-	explicit ArrayType(Location _location, bool _isString = false):
+	explicit ArrayType(DataLocation _location, bool _isString = false):
 		ReferenceType(_location),
 		m_arrayKind(_isString ? ArrayKind::String : ArrayKind::Bytes),
 		m_baseType(std::make_shared<FixedBytesType>(1))
 	{
 	}
 	/// Constructor for a dynamically sized array type ("type[]")
-	ArrayType(Location _location, TypePointer const& _baseType):
+	ArrayType(DataLocation _location, TypePointer const& _baseType):
 		ReferenceType(_location),
 		m_baseType(copyForLocationIfReference(_baseType))
 	{
 	}
 	/// Constructor for a fixed-size array type ("type[20]")
-	ArrayType(Location _location, TypePointer const& _baseType, u256 const& _length):
+	ArrayType(DataLocation _location, TypePointer const& _baseType, u256 const& _length):
 		ReferenceType(_location),
 		m_baseType(copyForLocationIfReference(_baseType)),
 		m_hasDynamicLength(false),
@@ -457,7 +459,7 @@ public:
 	TypePointer const& getBaseType() const { solAssert(!!m_baseType, ""); return m_baseType;}
 	u256 const& getLength() const { return m_length; }
 
-	TypePointer copyForLocation(Location _location, bool _isPointer) const override;
+	TypePointer copyForLocation(DataLocation _location, bool _isPointer) const override;
 
 private:
 	/// String is interpreted as a subtype of Bytes.
@@ -536,7 +538,7 @@ public:
 	virtual Category getCategory() const override { return Category::Struct; }
 	explicit StructType(StructDefinition const& _struct):
 		//@todo only storage until we have non-storage structs
-		ReferenceType(Location::Storage), m_struct(_struct) {}
+		ReferenceType(DataLocation::Storage), m_struct(_struct) {}
 	virtual bool isImplicitlyConvertibleTo(const Type& _convertTo) const override;
 	virtual TypePointer unaryOperatorResult(Token::Value _operator) const override;
 	virtual bool operator==(Type const& _other) const override;
@@ -547,7 +549,7 @@ public:
 
 	virtual MemberList const& getMembers() const override;
 
-	TypePointer copyForLocation(Location _location, bool _isPointer) const override;
+	TypePointer copyForLocation(DataLocation _location, bool _isPointer) const override;
 
 	std::pair<u256, unsigned> const& getStorageOffsetsOfMember(std::string const& _name) const;
 
@@ -639,8 +641,11 @@ public:
 	FunctionTypePointer externalFunctionType() const;
 	virtual TypePointer externalType() const override { return externalFunctionType(); }
 
+	/// Creates the type of a function.
 	explicit FunctionType(FunctionDefinition const& _function, bool _isInternal = true);
+	/// Creates the accessor function type of a state variable.
 	explicit FunctionType(VariableDeclaration const& _varDecl);
+	/// Creates the function type of an event.
 	explicit FunctionType(EventDefinition const& _event);
 	FunctionType(
 		strings const& _parameterTypes,