Merge pull request #1480 from chriseth/sol_refactorByteArray

Some cleanup concerning byte arrays.

libsolidity/ArrayUtils.cpp

@@ -388,10 +388,7 @@ void ArrayUtils::convertLengthToSize(ArrayType const& _arrayType, bool _pad) con
 {
 	if (_arrayType.getLocation() == ArrayType::Location::Storage)
 	{
-		if (_arrayType.isByteArray())
-			m_context << u256(31) << eth::Instruction::ADD
-				<< u256(32) << eth::Instruction::SWAP1 << eth::Instruction::DIV;
-		else if (_arrayType.getBaseType()->getStorageSize() <= 1)
+		if (_arrayType.getBaseType()->getStorageSize() <= 1)
 		{
 			unsigned baseBytes = _arrayType.getBaseType()->getStorageBytes();
 			if (baseBytes == 0)
@@ -464,34 +461,6 @@ void ArrayUtils::accessIndex(ArrayType const& _arrayType) const
 	m_context << eth::Instruction::STOP;
 
 	m_context << legalAccess;
-	// stack: <base_ref> <index>
-	if (_arrayType.isByteArray())
-		switch (location)
-		{
-		case ArrayType::Location::Storage:
-			// byte array index storage lvalue on stack (goal):
-			// <ref> <byte_number> = <base_ref + index / 32> <index % 32>
-			m_context << u256(32) << eth::Instruction::SWAP2;
-			CompilerUtils(m_context).computeHashStatic();
-			// stack: 32 index data_ref
-			m_context
-				<< eth::Instruction::DUP3 << eth::Instruction::DUP3
-				<< eth::Instruction::DIV << eth::Instruction::ADD
-			// stack: 32 index (data_ref + index / 32)
-				<< eth::Instruction::SWAP2 << eth::Instruction::SWAP1
-				<< eth::Instruction::MOD;
-			break;
-		case ArrayType::Location::CallData:
-			// no lvalue, just retrieve the value
-			m_context
-				<< eth::Instruction::ADD << eth::Instruction::CALLDATALOAD
-				<< ((u256(0xff) << (256 - 8)))  << eth::Instruction::AND;
-			break;
-		case ArrayType::Location::Memory:
-			solAssert(false, "Memory lvalues not yet implemented.");
-		}
-	else
-	{
 	// stack: <base_ref> <index>
 	m_context << eth::Instruction::SWAP1;
 	if (_arrayType.isDynamicallySized())
@@ -505,11 +474,19 @@ void ArrayUtils::accessIndex(ArrayType const& _arrayType) const
 	switch (location)
 	{
 	case ArrayType::Location::CallData:
+		if (!_arrayType.isByteArray())
 			m_context
-				<< eth::Instruction::SWAP1 << _arrayType.getBaseType()->getCalldataEncodedSize()
-				<< eth::Instruction::MUL << eth::Instruction::ADD;
+				<< eth::Instruction::SWAP1
+				<< _arrayType.getBaseType()->getCalldataEncodedSize()
+				<< eth::Instruction::MUL;
+		m_context << eth::Instruction::ADD;
 		if (_arrayType.getBaseType()->isValueType())
-				CompilerUtils(m_context).loadFromMemoryDynamic(*_arrayType.getBaseType(), true, true, false);
+			CompilerUtils(m_context).loadFromMemoryDynamic(
+				*_arrayType.getBaseType(),
+				true,
+				!_arrayType.isByteArray(),
+				false
+			);
 		break;
 	case ArrayType::Location::Storage:
 		m_context << eth::Instruction::SWAP1;
@@ -528,8 +505,9 @@ void ArrayUtils::accessIndex(ArrayType const& _arrayType) const
 				<< eth::Instruction::DIV << eth::Instruction::ADD
 			// stack: itemsPerSlot index (data_ref + index / itemsPerSlot)
 				<< eth::Instruction::SWAP2 << eth::Instruction::SWAP1
-					<< eth::Instruction::MOD
-					<< u256(byteSize) << eth::Instruction::MUL;
+				<< eth::Instruction::MOD;
+			if (byteSize != 1)
+				m_context << u256(byteSize) << eth::Instruction::MUL;
 		}
 		else
 		{
@@ -542,7 +520,6 @@ void ArrayUtils::accessIndex(ArrayType const& _arrayType) const
 		solAssert(false, "Memory lvalues not yet implemented.");
 	}
 }
-}
 
 void ArrayUtils::incrementByteOffset(unsigned _byteSize, unsigned _byteOffsetPosition, unsigned _storageOffsetPosition) const
 {

libsolidity/Compiler.cpp

@@ -254,7 +254,6 @@ void Compiler::appendCalldataUnpacker(TypePointers const& _typeParameters, bool
 
 void Compiler::appendReturnValuePacker(TypePointers const& _typeParameters)
 {
-	//@todo this can be also done more efficiently
 	unsigned dataOffset = 0;
 	unsigned stackDepth = 0;
 	for (TypePointer const& type: _typeParameters)
@@ -303,9 +302,6 @@ bool Compiler::visit(VariableDeclaration const& _variableDeclaration)
 bool Compiler::visit(FunctionDefinition const& _function)
 {
 	CompilerContext::LocationSetter locationSetter(m_context, _function);
-	//@todo to simplify this, the calling convention could by changed such that
-	// caller puts: [retarg0] ... [retargm] [return address] [arg0] ... [argn]
-	// although note that this reduces the size of the visible stack
 
 	m_context.startFunction(_function);
 

libsolidity/CompilerUtils.cpp

@@ -93,7 +93,7 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound
 		else
 		{
 			solAssert(type.getLocation() == ArrayType::Location::Storage, "Memory arrays not yet implemented.");
-			m_context << eth::Instruction::POP; //@todo
+			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
 			// jump to end if length is zero

libsolidity/LValue.cpp

@@ -225,7 +225,8 @@ void StorageItem::setToZero(SourceLocation const&, bool _removeReference) const
 	else if (m_dataType.getCategory() == Type::Category::Struct)
 	{
 		// stack layout: storage_key storage_offset
-		// @todo this can be improved for packed types
+		// @todo this can be improved: use StorageItem for non-value types, and just store 0 in
+		// all slots that contain value types later.
 		auto const& structType = dynamic_cast<StructType const&>(m_dataType);
 		for (auto const& member: structType.getMembers())
 		{
@@ -245,7 +246,6 @@ void StorageItem::setToZero(SourceLocation const&, bool _removeReference) const
 	else
 	{
 		solAssert(m_dataType.isValueType(), "Clearing of unsupported type requested: " + m_dataType.toString());
-		// @todo actually use offset
 		if (!_removeReference)
 			CompilerUtils(m_context).copyToStackTop(sizeOnStack(), sizeOnStack());
 		if (m_dataType.getStorageBytes() == 32)

libsolidity/Types.h

@@ -345,7 +345,7 @@ public:
 	explicit ArrayType(Location _location):
 		m_location(_location),
 		m_isByteArray(true),
-		m_baseType(std::make_shared<FixedBytesType>(8))
+		m_baseType(std::make_shared<FixedBytesType>(1))
 	{}
 	/// Constructor for a dynamically sized array type ("type[]")
 	ArrayType(Location _location, const TypePointer &_baseType):