Proper type promotion and conversion.

3 files changed, 67 insertions, 31 deletions

diff --git a/Compiler.cpp b/Compiler.cpp
index 654ecead..dfc351fa 100644
--- a/Compiler.cpp
+++ b/Compiler.cpp
@@ -273,7 +273,7 @@ bool Compiler::visit(Return& _return)
 	{
 		ExpressionCompiler::compileExpression(m_context, *expression);
 		VariableDeclaration const& firstVariable = *_return.getFunctionReturnParameters().getParameters().front();
-		ExpressionCompiler::cleanHigherOrderBitsIfNeeded(*expression->getType(), *firstVariable.getType());
+		ExpressionCompiler::appendTypeConversion(m_context, *expression->getType(), *firstVariable.getType());
 		int stackPosition = m_context.getStackPositionOfVariable(firstVariable);
 		m_context << eth::swapInstruction(stackPosition) << eth::Instruction::POP;
 	}
@@ -286,8 +286,9 @@ bool Compiler::visit(VariableDefinition& _variableDefinition)
 	if (Expression* expression = _variableDefinition.getExpression())
 	{
 		ExpressionCompiler::compileExpression(m_context, *expression);
-		ExpressionCompiler::cleanHigherOrderBitsIfNeeded(*expression->getType(),
-														 *_variableDefinition.getDeclaration().getType());
+		ExpressionCompiler::appendTypeConversion(m_context,
+												 *expression->getType(),
+												 *_variableDefinition.getDeclaration().getType());
 		int stackPosition = m_context.getStackPositionOfVariable(_variableDefinition.getDeclaration());
 		m_context << eth::swapInstruction(stackPosition) << eth::Instruction::POP;
 	}
diff --git a/ExpressionCompiler.cpp b/ExpressionCompiler.cpp
index a7d87221..0df74184 100644
--- a/ExpressionCompiler.cpp
+++ b/ExpressionCompiler.cpp
@@ -37,6 +37,13 @@ void ExpressionCompiler::compileExpression(CompilerContext& _context, Expression
 	_expression.accept(compiler);
 }
 
+void ExpressionCompiler::appendTypeConversion(CompilerContext& _context,
+											  Type const& _typeOnStack, Type const& _targetType)
+{
+	ExpressionCompiler compiler(_context);
+	compiler.appendTypeConversion(_typeOnStack, _targetType);
+}
+
 bool ExpressionCompiler::visit(Assignment& _assignment)
 {
 	m_currentLValue = nullptr;
@@ -44,7 +51,7 @@ bool ExpressionCompiler::visit(Assignment& _assignment)
 	Expression& rightHandSide = _assignment.getRightHandSide();
 	rightHandSide.accept(*this);
 	Type const& resultType = *_assignment.getType();
-	cleanHigherOrderBitsIfNeeded(*rightHandSide.getType(), resultType);
+	appendTypeConversion(*rightHandSide.getType(), resultType);
 	_assignment.getLeftHandSide().accept(*this);
 
 	Token::Value op = _assignment.getAssignmentOperator();
@@ -123,10 +130,21 @@ bool ExpressionCompiler::visit(BinaryOperation& _binaryOperation)
 	}
 	else
 	{
+		bool cleanupNeeded = false;
+		if (commonType.getCategory() == Type::Category::INTEGER)
+			if (Token::isCompareOp(op) || op == Token::DIV || op == Token::MOD)
+				cleanupNeeded = true;
+
 		leftExpression.accept(*this);
-		cleanHigherOrderBitsIfNeeded(*leftExpression.getType(), commonType);
+		if (cleanupNeeded)
+			appendHighBitsCleanup(dynamic_cast<IntegerType const&>(*leftExpression.getType()));
+		else
+			appendTypeConversion(*leftExpression.getType(), commonType);
 		rightExpression.accept(*this);
-		cleanHigherOrderBitsIfNeeded(*rightExpression.getType(), commonType);
+		if (cleanupNeeded)
+			appendHighBitsCleanup(dynamic_cast<IntegerType const&>(*leftExpression.getType()));
+		else
+			appendTypeConversion(*rightExpression.getType(), commonType);
 		if (Token::isCompareOp(op))
 			appendCompareOperatorCode(op, commonType);
 		else
@@ -146,7 +164,7 @@ bool ExpressionCompiler::visit(FunctionCall& _functionCall)
 			BOOST_THROW_EXCEPTION(InternalCompilerError());
 		Expression& firstArgument = *_functionCall.getArguments().front();
 		firstArgument.accept(*this);
-		cleanHigherOrderBitsIfNeeded(*firstArgument.getType(), *_functionCall.getType());
+		appendTypeConversion(*firstArgument.getType(), *_functionCall.getType());
 	}
 	else
 	{
@@ -163,7 +181,7 @@ bool ExpressionCompiler::visit(FunctionCall& _functionCall)
 		for (unsigned i = 0; i < arguments.size(); ++i)
 		{
 			arguments[i]->accept(*this);
-			cleanHigherOrderBitsIfNeeded(*arguments[i]->getType(),
+			appendTypeConversion(*arguments[i]->getType(),
 										 *function.getParameters()[i]->getType());
 		}
 
@@ -226,28 +244,6 @@ void ExpressionCompiler::endVisit(Literal& _literal)
 	}
 }
 
-void ExpressionCompiler::cleanHigherOrderBitsIfNeeded(Type const& _typeOnStack, Type const& _targetType)
-{
-	// If the type of one of the operands is extended, we need to remove all
-	// higher-order bits that we might have ignored in previous operations.
-	// @todo: store in the AST whether the operand might have "dirty" higher
-	// order bits
-
-	if (_typeOnStack == _targetType)
-		return;
-	if (_typeOnStack.getCategory() == Type::Category::INTEGER &&
-			_targetType.getCategory() == Type::Category::INTEGER)
-	{
-		//@todo
-	}
-	else
-	{
-		// If we get here, there is either an implementation missing to clean higher oder bits
-		// for non-integer types that are explicitly convertible or we got here in error.
-		BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Invalid type conversion requested."));
-	}
-}
-
 void ExpressionCompiler::appendAndOrOperatorCode(BinaryOperation& _binaryOperation)
 {
 	Token::Value const op = _binaryOperation.getOperator();
@@ -372,6 +368,37 @@ void ExpressionCompiler::appendShiftOperatorCode(Token::Value _operator)
 	}
 }
 
+void ExpressionCompiler::appendTypeConversion(Type const& _typeOnStack, Type const& _targetType)
+{
+	// If the type of one of the operands is extended, we need to remove all
+	// higher-order bits that we might have ignored in previous operations.
+	// @todo: store in the AST whether the operand might have "dirty" higher
+	// order bits
+
+	if (_typeOnStack == _targetType)
+		return;
+	if (_typeOnStack.getCategory() == Type::Category::INTEGER)
+	{
+		appendHighBitsCleanup(dynamic_cast<IntegerType const&>(_typeOnStack));
+	}
+	else
+	{
+		// All other types should not be convertible to non-equal types.
+		assert(!_typeOnStack.isExplicitlyConvertibleTo(_targetType));
+		assert(false);
+	}
+}
+
+void ExpressionCompiler::appendHighBitsCleanup(IntegerType const& _typeOnStack)
+{
+	if (_typeOnStack.getNumBits() == 256)
+		return;
+	else if (_typeOnStack.isSigned())
+		m_context << u256(_typeOnStack.getNumBits() / 8 - 1) << eth::Instruction::SIGNEXTEND;
+	else
+		m_context << ((u256(1) << _typeOnStack.getNumBits()) - 1) << eth::Instruction::AND;
+}
+
 void ExpressionCompiler::storeInLValue(Expression const& _expression)
 {
 	moveToLValue(_expression);
diff --git a/ExpressionCompiler.h b/ExpressionCompiler.h
index a930723c..7c731ec7 100644
--- a/ExpressionCompiler.h
+++ b/ExpressionCompiler.h
@@ -26,6 +26,8 @@ namespace dev {
 namespace solidity {
 
 class CompilerContext; // forward
+class Type; // forward
+class IntegerType; // forward
 
 /// Compiler for expressions, i.e. converts an AST tree whose root is an Expression into a stream
 /// of EVM instructions. It needs a compiler context that is the same for the whole compilation
@@ -37,7 +39,7 @@ public:
 	static void compileExpression(CompilerContext& _context, Expression& _expression);
 
 	/// Appends code to remove dirty higher order bits in case of an implicit promotion to a wider type.
-	static void cleanHigherOrderBitsIfNeeded(Type const& _typeOnStack, Type const& _targetType);
+	static void appendTypeConversion(CompilerContext& _context, Type const& _typeOnStack, Type const& _targetType);
 
 private:
 	ExpressionCompiler(CompilerContext& _compilerContext): m_currentLValue(nullptr), m_context(_compilerContext) {}
@@ -62,6 +64,12 @@ private:
 	void appendShiftOperatorCode(Token::Value _operator);
 	/// @}
 
+	/// Appends an implicit or explicit type conversion. For now this comprises only erasing
+	/// higher-order bits (@see appendHighBitCleanup) when widening integer types.
+	void appendTypeConversion(Type const& _typeOnStack, Type const& _targetType);
+	//// Appends code that cleans higher-order bits for integer types.
+	void appendHighBitsCleanup(IntegerType const& _typeOnStack);
+
 	/// Stores the value on top of the stack in the current lvalue and copies that value to the
 	/// top of the stack again
 	void storeInLValue(Expression const& _expression);