Merge pull request #3232 from ethereum/simplifyConstant

Simplify ConstantEvaluator.

5 files changed, 80 insertions, 62 deletions

diff --git a/libsolidity/analysis/ConstantEvaluator.cpp b/libsolidity/analysis/ConstantEvaluator.cpp
index 4d546e68..83f37f47 100644
--- a/libsolidity/analysis/ConstantEvaluator.cpp
+++ b/libsolidity/analysis/ConstantEvaluator.cpp
@@ -28,51 +28,42 @@ using namespace std;
 using namespace dev;
 using namespace dev::solidity;
 
-/// FIXME: this is pretty much a copy of TypeChecker::endVisit(BinaryOperation)
 void ConstantEvaluator::endVisit(UnaryOperation const& _operation)
 {
-	TypePointer const& subType = _operation.subExpression().annotation().type;
-	if (!dynamic_cast<RationalNumberType const*>(subType.get()))
-		m_errorReporter.fatalTypeError(_operation.subExpression().location(), "Invalid constant expression.");
-	TypePointer t = subType->unaryOperatorResult(_operation.getOperator());
-	_operation.annotation().type = t;
+	auto sub = type(_operation.subExpression());
+	if (sub)
+		setType(_operation, sub->unaryOperatorResult(_operation.getOperator()));
 }
 
-/// FIXME: this is pretty much a copy of TypeChecker::endVisit(BinaryOperation)
 void ConstantEvaluator::endVisit(BinaryOperation const& _operation)
 {
-	TypePointer const& leftType = _operation.leftExpression().annotation().type;
-	TypePointer const& rightType = _operation.rightExpression().annotation().type;
-	if (!dynamic_cast<RationalNumberType const*>(leftType.get()))
-		m_errorReporter.fatalTypeError(_operation.leftExpression().location(), "Invalid constant expression.");
-	if (!dynamic_cast<RationalNumberType const*>(rightType.get()))
-		m_errorReporter.fatalTypeError(_operation.rightExpression().location(), "Invalid constant expression.");
-	TypePointer commonType = leftType->binaryOperatorResult(_operation.getOperator(), rightType);
-	if (!commonType)
+	auto left = type(_operation.leftExpression());
+	auto right = type(_operation.rightExpression());
+	if (left && right)
 	{
-		m_errorReporter.typeError(
-			_operation.location(),
-			"Operator " +
-			string(Token::toString(_operation.getOperator())) +
-			" not compatible with types " +
-			leftType->toString() +
-			" and " +
-			rightType->toString()
+		auto commonType = left->binaryOperatorResult(_operation.getOperator(), right);
+		if (!commonType)
+			m_errorReporter.fatalTypeError(
+				_operation.location(),
+				"Operator " +
+				string(Token::toString(_operation.getOperator())) +
+				" not compatible with types " +
+				left->toString() +
+				" and " +
+				right->toString()
+			);
+		setType(
+			_operation,
+			Token::isCompareOp(_operation.getOperator()) ?
+			make_shared<BoolType>() :
+			commonType
 		);
-		commonType = leftType;
 	}
-	_operation.annotation().commonType = commonType;
-	_operation.annotation().type =
-		Token::isCompareOp(_operation.getOperator()) ?
-		make_shared<BoolType>() :
-		commonType;
 }
 
 void ConstantEvaluator::endVisit(Literal const& _literal)
 {
-	_literal.annotation().type = Type::forLiteral(_literal);
-	if (!_literal.annotation().type)
-		m_errorReporter.fatalTypeError(_literal.location(), "Invalid literal value.");
+	setType(_literal, Type::forLiteral(_literal));
 }
 
 void ConstantEvaluator::endVisit(Identifier const& _identifier)
@@ -81,18 +72,34 @@ void ConstantEvaluator::endVisit(Identifier const& _identifier)
 	if (!variableDeclaration)
 		return;
 	if (!variableDeclaration->isConstant())
-		m_errorReporter.fatalTypeError(_identifier.location(), "Identifier must be declared constant.");
+		return;
 
-	ASTPointer<Expression> value = variableDeclaration->value();
+	ASTPointer<Expression> const& value = variableDeclaration->value();
 	if (!value)
-		m_errorReporter.fatalTypeError(_identifier.location(), "Constant identifier declaration must have a constant value.");
-
-	if (!value->annotation().type)
+		return;
+	else if (!m_types->count(value.get()))
 	{
 		if (m_depth > 32)
 			m_errorReporter.fatalTypeError(_identifier.location(), "Cyclic constant definition (or maximum recursion depth exhausted).");
-		ConstantEvaluator e(*value, m_errorReporter, m_depth + 1);
+		ConstantEvaluator(m_errorReporter, m_depth + 1, m_types).evaluate(*value);
 	}
 
-	_identifier.annotation().type = value->annotation().type;
+	setType(_identifier, type(*value));
+}
+
+void ConstantEvaluator::setType(ASTNode const& _node, TypePointer const& _type)
+{
+	if (_type && _type->category() == Type::Category::RationalNumber)
+		(*m_types)[&_node] = _type;
+}
+
+TypePointer ConstantEvaluator::type(ASTNode const& _node)
+{
+	return (*m_types)[&_node];
+}
+
+TypePointer ConstantEvaluator::evaluate(Expression const& _expr)
+{
+	_expr.accept(*this);
+	return type(_expr);
 }
diff --git a/libsolidity/analysis/ConstantEvaluator.h b/libsolidity/analysis/ConstantEvaluator.h
index 6725d610..77a357b6 100644
--- a/libsolidity/analysis/ConstantEvaluator.h
+++ b/libsolidity/analysis/ConstantEvaluator.h
@@ -38,22 +38,32 @@ class TypeChecker;
 class ConstantEvaluator: private ASTConstVisitor
 {
 public:
-	ConstantEvaluator(Expression const& _expr, ErrorReporter& _errorReporter, size_t _newDepth = 0):
+	ConstantEvaluator(
+		ErrorReporter& _errorReporter,
+		size_t _newDepth = 0,
+		std::shared_ptr<std::map<ASTNode const*, TypePointer>> _types = std::make_shared<std::map<ASTNode const*, TypePointer>>()
+	):
 		m_errorReporter(_errorReporter),
-		m_depth(_newDepth)
+		m_depth(_newDepth),
+		m_types(_types)
 	{
-		_expr.accept(*this);
 	}
 
+	TypePointer evaluate(Expression const& _expr);
+
 private:
 	virtual void endVisit(BinaryOperation const& _operation);
 	virtual void endVisit(UnaryOperation const& _operation);
 	virtual void endVisit(Literal const& _literal);
 	virtual void endVisit(Identifier const& _identifier);
 
+	void setType(ASTNode const& _node, TypePointer const& _type);
+	TypePointer type(ASTNode const& _node);
+
 	ErrorReporter& m_errorReporter;
 	/// Current recursion depth.
-	size_t m_depth;
+	size_t m_depth = 0;
+	std::shared_ptr<std::map<ASTNode const*, TypePointer>> m_types;
 };
 
 }
diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp
index f22c95cc..9eee16af 100644
--- a/libsolidity/analysis/ReferencesResolver.cpp
+++ b/libsolidity/analysis/ReferencesResolver.cpp
@@ -146,11 +146,12 @@ void ReferencesResolver::endVisit(ArrayTypeName const& _typeName)
 		fatalTypeError(_typeName.baseType().location(), "Illegal base type of storage size zero for array.");
 	if (Expression const* length = _typeName.length())
 	{
-		if (!length->annotation().type)
-			ConstantEvaluator e(*length, m_errorReporter);
-		auto const* lengthType = dynamic_cast<RationalNumberType const*>(length->annotation().type.get());
+		TypePointer lengthTypeGeneric = length->annotation().type;
+		if (!lengthTypeGeneric)
+			lengthTypeGeneric = ConstantEvaluator(m_errorReporter).evaluate(*length);
+		RationalNumberType const* lengthType = dynamic_cast<RationalNumberType const*>(lengthTypeGeneric.get());
 		if (!lengthType || !lengthType->mobileType())
-			fatalTypeError(length->location(), "Invalid array length, expected integer literal.");
+			fatalTypeError(length->location(), "Invalid array length, expected integer literal or constant expression.");
 		else if (lengthType->isFractional())
 			fatalTypeError(length->location(), "Array with fractional length specified.");
 		else if (lengthType->isNegative())
diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h
index 635279ab..a54e4e09 100644
--- a/libsolidity/ast/Types.h
+++ b/libsolidity/ast/Types.h
@@ -257,7 +257,7 @@ public:
 	}
 	virtual u256 literalValue(Literal const*) const
 	{
-		solAssert(false, "Literal value requested for type without literals.");
+		solAssert(false, "Literal value requested for type without literals: " + toString(false));
 	}
 
 	/// @returns a (simpler) type that is encoded in the same way for external function calls.
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index 39dba0de..8f58dcb1 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -2109,7 +2109,7 @@ BOOST_AUTO_TEST_CASE(array_with_nonconstant_length)
 			function f(uint a) public { uint8[a] x; }
 		}
 	)";
-	CHECK_ERROR(text, TypeError, "Identifier must be declared constant.");
+	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal or constant expression.");
 }
 
 BOOST_AUTO_TEST_CASE(array_with_negative_length)
@@ -4398,7 +4398,7 @@ BOOST_AUTO_TEST_CASE(invalid_array_declaration_with_signed_fixed_type)
 			}
 		}
 	)";
-	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal.");
+	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal or constant expression.");
 }
 
 BOOST_AUTO_TEST_CASE(invalid_array_declaration_with_unsigned_fixed_type)
@@ -4410,7 +4410,7 @@ BOOST_AUTO_TEST_CASE(invalid_array_declaration_with_unsigned_fixed_type)
 			}
 		}
 	)";
-	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal.");
+	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal or constant expression.");
 }
 
 BOOST_AUTO_TEST_CASE(rational_to_bytes_implicit_conversion)
@@ -7254,7 +7254,7 @@ BOOST_AUTO_TEST_CASE(array_length_too_large)
 			uint[8**90] ids;
 		}
 	)";
-	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal.");
+	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal or constant expression.");
 }
 
 BOOST_AUTO_TEST_CASE(array_length_not_convertible_to_integer)
@@ -7264,7 +7264,7 @@ BOOST_AUTO_TEST_CASE(array_length_not_convertible_to_integer)
 			uint[true] ids;
 		}
 	)";
-	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal.");
+	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal or constant expression.");
 }
 
 BOOST_AUTO_TEST_CASE(array_length_constant_var)
@@ -7286,7 +7286,7 @@ BOOST_AUTO_TEST_CASE(array_length_non_integer_constant_var)
 			uint[LEN] ids;
 		}
 	)";
-	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal.");
+	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal or constant expression.");
 }
 
 BOOST_AUTO_TEST_CASE(array_length_cannot_be_function)
@@ -7297,7 +7297,7 @@ BOOST_AUTO_TEST_CASE(array_length_cannot_be_function)
 			uint[f] ids;
 		}
 	)";
-	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal.");
+	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal or constant expression.");
 }
 
 BOOST_AUTO_TEST_CASE(array_length_can_be_recursive_constant)
@@ -7321,7 +7321,7 @@ BOOST_AUTO_TEST_CASE(array_length_cannot_be_function_call)
 			uint[LEN] ids;
 		}
 	)";
-	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal.");
+	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal or constant expression.");
 }
 
 BOOST_AUTO_TEST_CASE(array_length_const_cannot_be_fractional)
@@ -7370,7 +7370,7 @@ BOOST_AUTO_TEST_CASE(array_length_cannot_be_constant_function_parameter)
 			}
 		}
 	)";
-	CHECK_ERROR(text, TypeError, "Constant identifier declaration must have a constant value.");
+	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal or constant expression.");
 }
 
 BOOST_AUTO_TEST_CASE(array_length_with_cyclic_constant)
@@ -7409,7 +7409,7 @@ BOOST_AUTO_TEST_CASE(array_length_with_pure_functions)
 			uint[LEN] ids;
 		}
 	)";
-	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal.");
+	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal or constant expression.");
 }
 
 BOOST_AUTO_TEST_CASE(array_length_invalid_expression)
@@ -7419,25 +7419,25 @@ BOOST_AUTO_TEST_CASE(array_length_invalid_expression)
 			uint[-true] ids;
 		}
 	)";
-	CHECK_ERROR(text, TypeError, "Invalid constant expression.");
+	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal or constant expression.");
 	text = R"(
 		contract C {
 			uint[true/1] ids;
 		}
 	)";
-	CHECK_ERROR(text, TypeError, "Invalid constant expression.");
+	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal or constant expression.");
 	text = R"(
 		contract C {
 			uint[1/true] ids;
 		}
 	)";
-	CHECK_ERROR(text, TypeError, "Invalid constant expression.");
+	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal or constant expression.");
 	text = R"(
 		contract C {
 			uint[1.111111E1111111111111] ids;
 		}
 	)";
-	CHECK_ERROR(text, TypeError, "Invalid literal value.");
+	CHECK_ERROR(text, TypeError, "Invalid array length, expected integer literal or constant expression.");
 	text = R"(
 		contract C {
 			uint[3/0] ids;