Merge pull request #6283 from ethereum/result-refactoring-merger

Result<T> improvements

libdevcore/Result.h

@@ -30,7 +30,7 @@ namespace dev
 /// Result<bool> check()
 /// {
 ///		if (false)
-///			return Result<bool>("Error message.")
+///			return Result<bool>::err("Error message.")
 ///		return true;
 /// }
 ///
@@ -39,8 +39,17 @@ template <class ResultType>
 class Result
 {
 public:
+	/// Constructs a result with _value and an empty message.
+	/// This is meant to be called with valid results. Please use
+	/// the static err() member function to signal an error.
 	Result(ResultType _value): Result(_value, std::string{}) {}
-	Result(std::string _message): Result(ResultType{}, std::move(_message)) {}
+
+	/// Constructs a result with a default-constructed value and an
+	/// error message.
+	static Result<ResultType> err(std::string _message)
+	{
+		return Result{ResultType{}, std::move(_message)};
+	}
 
 	/// @{
 	/// @name Wrapper functions
@@ -53,6 +62,16 @@ public:
 	/// @returns the error message (can be empty).
 	std::string const& message() const { return m_message; }
 
+	/// Merges _other into this using the _merger
+	/// and appends the error messages. Meant to be called
+	/// with logical operators like logical_and, etc.
+	template<typename F>
+	void merge(Result<ResultType> const& _other, F _merger)
+	{
+		m_value = _merger(m_value, _other.get());
+		m_message += _other.message();
+	}
+
 private:
 	explicit Result(ResultType _value, std::string _message):
 		m_value(std::move(_value)),

libsolidity/ast/Types.cpp

@@ -129,10 +129,10 @@ bool fitsPrecisionBase2(bigint const& _mantissa, uint32_t _expBase2)
 BoolResult fitsIntegerType(bigint const& _value, IntegerType const& _type)
 {
 	if (_value < 0 && !_type.isSigned())
-		return BoolResult{std::string("Cannot implicitly convert signed literal to unsigned type.")};
+		return BoolResult::err("Cannot implicitly convert signed literal to unsigned type.");
 
 	if (_type.minValue() > _value || _value > _type.maxValue())
-		return BoolResult{"Literal is too large to fit in " + _type.toString(false) + "."};
+		return BoolResult::err("Literal is too large to fit in " + _type.toString(false) + ".");
 
 	return true;
 }
@@ -535,7 +535,7 @@ TypeResult AddressType::unaryOperatorResult(Token _operator) const
 TypeResult AddressType::binaryOperatorResult(Token _operator, TypePointer const& _other) const
 {
 	if (!TokenTraits::isCompareOp(_operator))
-		return TypeResult{"Arithmetic operations on addresses are not supported. Convert to integer first before using them."};
+		return TypeResult::err("Arithmetic operations on addresses are not supported. Convert to integer first before using them.");
 
 	return Type::commonType(shared_from_this(), _other);
 }
@@ -638,7 +638,7 @@ TypeResult IntegerType::unaryOperatorResult(Token _operator) const
 		_operator == Token::Dec || _operator == Token::BitNot)
 		return TypeResult{shared_from_this()};
 	else
-		return TypeResult{""};
+		return TypeResult::err("");
 }
 
 bool IntegerType::operator==(Type const& _other) const
@@ -700,7 +700,7 @@ TypeResult IntegerType::binaryOperatorResult(Token _operator, TypePointer const&
 	if (auto intType = dynamic_pointer_cast<IntegerType const>(commonType))
 	{
 		if (Token::Exp == _operator && intType->isSigned())
-			return TypeResult{"Exponentiation is not allowed for signed integer types."};
+			return TypeResult::err("Exponentiation is not allowed for signed integer types.");
 	}
 	else if (auto fixType = dynamic_pointer_cast<FixedPointType const>(commonType))
 		if (Token::Exp == _operator)
@@ -729,7 +729,7 @@ BoolResult FixedPointType::isImplicitlyConvertibleTo(Type const& _convertTo) con
 	{
 		FixedPointType const& convertTo = dynamic_cast<FixedPointType const&>(_convertTo);
 		if (convertTo.fractionalDigits() < m_fractionalDigits)
-			return BoolResult{std::string("Too many fractional digits.")};
+			return BoolResult::err("Too many fractional digits.");
 		if (convertTo.numBits() < m_totalBits)
 			return false;
 		else
@@ -1145,7 +1145,7 @@ TypeResult RationalNumberType::binaryOperatorResult(Token _operator, TypePointer
 				uint32_t absExp = bigint(abs(exp)).convert_to<uint32_t>();
 
 				if (!fitsPrecisionExp(abs(m_value.numerator()), absExp) || !fitsPrecisionExp(abs(m_value.denominator()), absExp))
-					return TypeResult{"Precision of rational constants is limited to 4096 bits."};
+					return TypeResult::err("Precision of rational constants is limited to 4096 bits.");
 
 				static auto const optimizedPow = [](bigint const& _base, uint32_t _exponent) -> bigint {
 					if (_base == 1)
@@ -1226,7 +1226,7 @@ TypeResult RationalNumberType::binaryOperatorResult(Token _operator, TypePointer
 
 		// verify that numerator and denominator fit into 4096 bit after every operation
 		if (value.numerator() != 0 && max(mostSignificantBit(abs(value.numerator())), mostSignificantBit(abs(value.denominator()))) > 4096)
-			return TypeResult{"Precision of rational constants is limited to 4096 bits."};
+			return TypeResult::err("Precision of rational constants is limited to 4096 bits.");
 
 		return TypeResult(make_shared<RationalNumberType>(value));
 	}

test/libsolidity/SolidityTypes.cpp

@@ -243,6 +243,50 @@ BOOST_AUTO_TEST_CASE(encoded_sizes)
 	BOOST_CHECK_EQUAL(twoDimArray.calldataEncodedSize(false), 9 * 3 * 32);
 }
 
+BOOST_AUTO_TEST_CASE(helper_bool_result)
+{
+	BoolResult r1{true};
+	BoolResult r2 = BoolResult::err("Failure.");
+	r1.merge(r2, logical_and<bool>());
+	BOOST_REQUIRE_EQUAL(r1.get(), false);
+	BOOST_REQUIRE_EQUAL(r1.message(), "Failure.");
+
+	BoolResult r3{false};
+	BoolResult r4{true};
+	r3.merge(r4, logical_and<bool>());
+	BOOST_REQUIRE_EQUAL(r3.get(), false);
+	BOOST_REQUIRE_EQUAL(r3.message(), "");
+
+	BoolResult r5{true};
+	BoolResult r6{true};
+	r5.merge(r6, logical_and<bool>());
+	BOOST_REQUIRE_EQUAL(r5.get(), true);
+	BOOST_REQUIRE_EQUAL(r5.message(), "");
+
+	BoolResult r7{true};
+	// Attention: this will implicitly convert to bool.
+	BoolResult r8{"true"};
+	r7.merge(r8, logical_and<bool>());
+	BOOST_REQUIRE_EQUAL(r7.get(), true);
+	BOOST_REQUIRE_EQUAL(r7.message(), "");
+}
+
+BOOST_AUTO_TEST_CASE(helper_string_result)
+{
+	using StringResult = Result<string>;
+
+	StringResult r1{string{"Success"}};
+	StringResult r2 = StringResult::err("Failure");
+
+	BOOST_REQUIRE_EQUAL(r1.get(), "Success");
+	BOOST_REQUIRE_EQUAL(r2.get(), "");
+
+	r1.merge(r2, [](string const&, string const& _rhs) { return _rhs; });
+
+	BOOST_REQUIRE_EQUAL(r1.get(), "");
+	BOOST_REQUIRE_EQUAL(r1.message(), "Failure");
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }