Merge remote-tracking branch 'ethereum/develop' into sol_overloadingFunctions

7 files changed, 87 insertions, 99 deletions

diff --git a/AST.cpp b/AST.cpp
index 1c1983d9..acf39934 100644
--- a/AST.cpp
+++ b/AST.cpp
@@ -451,20 +451,16 @@ void VariableDeclaration::checkTypeRequirements()
 		if ((m_type && !m_type->isValueType()) || !m_value)
 			BOOST_THROW_EXCEPTION(createTypeError("Unitialized \"constant\" variable."));
 	}
-	if (!m_value)
-		return;
 	if (m_type)
 	{
-		m_value->expectType(*m_type);
-		if (m_isStateVariable && !m_type->externalType() && getVisibility() >= Visibility::Public)
-			BOOST_THROW_EXCEPTION(createTypeError("Internal type is not allowed for state variables."));
-
-		if (!FunctionType(*this).externalType())
-			BOOST_THROW_EXCEPTION(createTypeError("Internal type is not allowed for public state variables."));
+		if (m_value)
+			m_value->expectType(*m_type);
 	}
 	else
 	{
-		// no type declared and no previous assignment, infer the type
+		if (!m_value)
+			// This feature might be extended in the future.
+			BOOST_THROW_EXCEPTION(createTypeError("Assignment necessary for type detection."));
 		m_value->checkTypeRequirements(nullptr);
 
 		TypePointer type = m_value->getType();
@@ -479,6 +475,8 @@ void VariableDeclaration::checkTypeRequirements()
 			BOOST_THROW_EXCEPTION(createTypeError("Variable cannot have void type."));
 		m_type = type;
 	}
+	if (m_isStateVariable && getVisibility() >= Visibility::Public && !FunctionType(*this).externalType())
+		BOOST_THROW_EXCEPTION(createTypeError("Internal type is not allowed for public state variables."));
 }
 
 bool VariableDeclaration::isExternalFunctionParameter() const
diff --git a/ArrayUtils.cpp b/ArrayUtils.cpp
index 58031390..448e4da2 100644
--- a/ArrayUtils.cpp
+++ b/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)
@@ -465,82 +462,62 @@ void ArrayUtils::accessIndex(ArrayType const& _arrayType) const
 
 	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;
+	m_context << eth::Instruction::SWAP1;
+	if (_arrayType.isDynamicallySized())
+	{
+		if (location == ArrayType::Location::Storage)
 			CompilerUtils(m_context).computeHashStatic();
-			// stack: 32 index data_ref
+		else if (location == ArrayType::Location::Memory)
+			m_context << u256(32) << eth::Instruction::ADD;
+	}
+	// stack: <index> <data_ref>
+	switch (location)
+	{
+	case ArrayType::Location::CallData:
+		if (!_arrayType.isByteArray())
+			m_context
+				<< 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,
+				!_arrayType.isByteArray(),
+				false
+			);
+		break;
+	case ArrayType::Location::Storage:
+		m_context << eth::Instruction::SWAP1;
+		if (_arrayType.getBaseType()->getStorageBytes() <= 16)
+		{
+			// stack: <data_ref> <index>
+			// goal:
+			// <ref> <byte_number> = <base_ref + index / itemsPerSlot> <(index % itemsPerSlot) * byteSize>
+			unsigned byteSize = _arrayType.getBaseType()->getStorageBytes();
+			solAssert(byteSize != 0, "");
+			unsigned itemsPerSlot = 32 / byteSize;
+			m_context << u256(itemsPerSlot) << eth::Instruction::SWAP2;
+			// stack: itemsPerSlot index data_ref
 			m_context
 				<< eth::Instruction::DUP3 << eth::Instruction::DUP3
 				<< eth::Instruction::DIV << eth::Instruction::ADD
-			// stack: 32 index (data_ref + index / 32)
+			// stack: itemsPerSlot index (data_ref + index / itemsPerSlot)
 				<< 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())
-		{
-			if (location == ArrayType::Location::Storage)
-				CompilerUtils(m_context).computeHashStatic();
-			else if (location == ArrayType::Location::Memory)
-				m_context << u256(32) << eth::Instruction::ADD;
+			if (byteSize != 1)
+				m_context << u256(byteSize) << eth::Instruction::MUL;
 		}
-		// stack: <index> <data_ref>
-		switch (location)
+		else
 		{
-		case ArrayType::Location::CallData:
-			m_context
-				<< eth::Instruction::SWAP1 << _arrayType.getBaseType()->getCalldataEncodedSize()
-				<< eth::Instruction::MUL << eth::Instruction::ADD;
-			if (_arrayType.getBaseType()->isValueType())
-				CompilerUtils(m_context).loadFromMemoryDynamic(*_arrayType.getBaseType(), true, true, false);
-			break;
-		case ArrayType::Location::Storage:
-			m_context << eth::Instruction::SWAP1;
-			if (_arrayType.getBaseType()->getStorageBytes() <= 16)
-			{
-				// stack: <data_ref> <index>
-				// goal:
-				// <ref> <byte_number> = <base_ref + index / itemsPerSlot> <(index % itemsPerSlot) * byteSize>
-				unsigned byteSize = _arrayType.getBaseType()->getStorageBytes();
-				solAssert(byteSize != 0, "");
-				unsigned itemsPerSlot = 32 / byteSize;
-				m_context << u256(itemsPerSlot) << eth::Instruction::SWAP2;
-				// stack: itemsPerSlot index data_ref
-				m_context
-					<< eth::Instruction::DUP3 << eth::Instruction::DUP3
-					<< 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;
-			}
-			else
-			{
-				if (_arrayType.getBaseType()->getStorageSize() != 1)
-					m_context << _arrayType.getBaseType()->getStorageSize() << eth::Instruction::MUL;
-				m_context << eth::Instruction::ADD << u256(0);
-			}
-			break;
-		case ArrayType::Location::Memory:
-			solAssert(false, "Memory lvalues not yet implemented.");
+			if (_arrayType.getBaseType()->getStorageSize() != 1)
+				m_context << _arrayType.getBaseType()->getStorageSize() << eth::Instruction::MUL;
+			m_context << eth::Instruction::ADD << u256(0);
 		}
+		break;
+	case ArrayType::Location::Memory:
+		solAssert(false, "Memory lvalues not yet implemented.");
 	}
 }
 
diff --git a/Compiler.cpp b/Compiler.cpp
index a8d0a591..83954922 100644
--- a/Compiler.cpp
+++ b/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);
 
diff --git a/CompilerUtils.cpp b/CompilerUtils.cpp
index 45495114..8d3e9d2a 100644
--- a/CompilerUtils.cpp
+++ b/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
diff --git a/LValue.cpp b/LValue.cpp
index fc990b0a..bb68a663 100644
--- a/LValue.cpp
+++ b/LValue.cpp
@@ -107,6 +107,11 @@ void StorageItem::retrieveValue(SourceLocation const&, bool _remove) const
 			<< u256(0x100) << eth::Instruction::EXP << eth::Instruction::SWAP1 << eth::Instruction::DIV;
 		if (m_dataType.getCategory() == Type::Category::FixedBytes)
 			m_context << (u256(0x1) << (256 - 8 * m_dataType.getStorageBytes())) << eth::Instruction::MUL;
+		else if (
+			m_dataType.getCategory() == Type::Category::Integer &&
+			dynamic_cast<IntegerType const&>(m_dataType).isSigned()
+		)
+			m_context << u256(m_dataType.getStorageBytes() - 1) << eth::Instruction::SIGNEXTEND;
 		else
 			m_context << ((u256(0x1) << (8 * m_dataType.getStorageBytes())) - 1) << eth::Instruction::AND;
 	}
@@ -148,6 +153,17 @@ void StorageItem::storeValue(Type const& _sourceType, SourceLocation const& _loc
 				m_context
 					<< (u256(0x1) << (256 - 8 * dynamic_cast<FixedBytesType const&>(m_dataType).getNumBytes()))
 					<< eth::Instruction::SWAP1 << eth::Instruction::DIV;
+			else if (
+				m_dataType.getCategory() == Type::Category::Integer &&
+				dynamic_cast<IntegerType const&>(m_dataType).isSigned()
+			)
+				// remove the higher order bits
+				m_context
+					<< (u256(1) << (8 * (32 - m_dataType.getStorageBytes())))
+					<< eth::Instruction::SWAP1
+					<< eth::Instruction::DUP2
+					<< eth::Instruction::MUL
+					<< eth::Instruction::DIV;
 			m_context  << eth::Instruction::MUL << eth::Instruction::OR;
 			// stack: value storage_ref updated_value
 			m_context << eth::Instruction::SWAP1 << eth::Instruction::SSTORE;
@@ -225,7 +241,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 +262,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)
diff --git a/Parser.cpp b/Parser.cpp
index 5c7676df..43571314 100644
--- a/Parser.cpp
+++ b/Parser.cpp
@@ -472,17 +472,18 @@ ASTPointer<TypeName> Parser::parseTypeName(bool _allowVar)
 	else
 		BOOST_THROW_EXCEPTION(createParserError("Expected type name"));
 
-	// Parse "[...]" postfixes for arrays.
-	while (m_scanner->getCurrentToken() == Token::LBrack)
-	{
-		m_scanner->next();
-		ASTPointer<Expression> length;
-		if (m_scanner->getCurrentToken() != Token::RBrack)
-			length = parseExpression();
-		nodeFactory.markEndPosition();
-		expectToken(Token::RBrack);
-		type = nodeFactory.createNode<ArrayTypeName>(type, length);
-	}
+	if (type)
+		// Parse "[...]" postfixes for arrays.
+		while (m_scanner->getCurrentToken() == Token::LBrack)
+		{
+			m_scanner->next();
+			ASTPointer<Expression> length;
+			if (m_scanner->getCurrentToken() != Token::RBrack)
+				length = parseExpression();
+			nodeFactory.markEndPosition();
+			expectToken(Token::RBrack);
+			type = nodeFactory.createNode<ArrayTypeName>(type, length);
+		}
 	return type;
 }
 
diff --git a/Types.h b/Types.h
index 446737fa..97c78f3c 100644
--- a/Types.h
+++ b/Types.h
@@ -371,7 +371,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):