[cond-expr] fixup according to code review

4 files changed, 24 insertions, 6 deletions

diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index f2056474..f3840675 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -765,7 +765,7 @@ bool TypeChecker::visit(Conditional const& _conditional)
 					" doesn't match false expression's type " +
 					falseType->toString() +
 					"."
-					);
+			);
 
 		_conditional.annotation().type = trueType;
 		_conditional.annotation().isLValue = true;
@@ -781,6 +781,7 @@ bool TypeChecker::visit(Conditional const& _conditional)
 		// we fake it as an equal operator, but any other comparison operator can work.
 		TypePointer commonType = trueType->binaryOperatorResult(Token::Equal, falseType);
 		if (!commonType)
+		{
 			typeError(
 					_conditional.location(),
 					"True expression's type " +
@@ -788,7 +789,11 @@ bool TypeChecker::visit(Conditional const& _conditional)
 					" doesn't match false expression's type " +
 					falseType->toString() +
 					"."
-					);
+			);
+			// even we can't find a common type, we have to set a type here,
+			// otherwise the upper statement will not be able to check the type.
+			commonType = trueType;
+		}
 
 		_conditional.annotation().type = commonType;
 	}
diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index 5e9c6920..9536c727 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -182,10 +182,12 @@ bool ExpressionCompiler::visit(Conditional const& _condition)
 	_condition.condition().accept(*this);
 	eth::AssemblyItem trueTag = m_context.appendConditionalJump();
 	_condition.falseExpression().accept(*this);
+	utils().convertType(*_condition.falseExpression().annotation().type, *_condition.annotation().type);
 	eth::AssemblyItem endTag = m_context.appendJumpToNew();
 	m_context << trueTag;
-	m_context.adjustStackOffset(-1);
+	m_context.adjustStackOffset(-_condition.annotation().type->sizeOnStack());
 	_condition.trueExpression().accept(*this);
+	utils().convertType(*_condition.trueExpression().annotation().type, *_condition.annotation().type);
 	m_context << endTag;
 	return false;
 }
diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp
index cc06eb87..d9ec1a49 100644
--- a/libsolidity/parsing/Parser.cpp
+++ b/libsolidity/parsing/Parser.cpp
@@ -947,7 +947,7 @@ ASTPointer<Expression> Parser::parseExpression(
 		expectToken(Token::Colon);
 		ASTPointer<Expression> falseExpression = parseExpression();
 		ASTNodeFactory nodeFactory(*this, expression);
-		nodeFactory.setEndPositionFromNode(falseExpression); // TODO:
+		nodeFactory.setEndPositionFromNode(falseExpression);
 		return nodeFactory.createNode<Conditional>(expression, trueExpression, falseExpression);
 	}
 	else
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 46199578..3b9b8449 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -128,7 +128,6 @@ BOOST_AUTO_TEST_CASE(conditional_expression_multiple)
 	BOOST_CHECK(callContractFunction("f(uint256)", u256(40)) == toBigEndian(u256(10)));
 }
 
-/*
 BOOST_AUTO_TEST_CASE(conditional_expression_as_left_value)
 {
 	char const* sourceCode = R"(
@@ -144,7 +143,19 @@ BOOST_AUTO_TEST_CASE(conditional_expression_as_left_value)
 	BOOST_CHECK(callContractFunction("f(uint256)", u256(20)) == toBigEndian(u256(3)));
 	BOOST_CHECK(callContractFunction("f(uint256)", u256(5)) == toBigEndian(u256(1)));
 }
-*/
+
+BOOST_AUTO_TEST_CASE(conditional_expression_with_return_values)
+{
+	char const* sourceCode = R"(
+		contract test {
+			function f(bool cond, uint v) returns (uint a, uint b) {
+				cond ? a = v : b = v;
+ 			}
+		})";
+	compileAndRun(sourceCode);
+	BOOST_CHECK(callContractFunction("f(bool,uint256)", true, u256(20)) == encodeArgs(u256(20), u256(0)));
+	BOOST_CHECK(callContractFunction("f(bool,uint256)", false, u256(20)) == encodeArgs(u256(0), u256(20)));
+}
 
 BOOST_AUTO_TEST_CASE(recursive_calls)
 {