Change function type to include and propagate payable and constant modifier.

8 files changed, 123 insertions, 62 deletions

diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index c1781397..c948b90b 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -349,7 +349,7 @@ void TypeChecker::endVisit(InheritanceSpecifier const& _inheritance)
 		typeError(_inheritance.location(), "Libraries cannot be inherited from.");
 
 	auto const& arguments = _inheritance.arguments();
-	TypePointers parameterTypes = ContractType(*base).constructorType()->parameterTypes();
+	TypePointers parameterTypes = ContractType(*base).newExpressionType()->parameterTypes();
 	if (!arguments.empty() && parameterTypes.size() != arguments.size())
 	{
 		typeError(
@@ -1264,15 +1264,7 @@ void TypeChecker::endVisit(NewExpression const& _newExpression)
 				"Circular reference for contract creation (cannot create instance of derived or same contract)."
 			);
 
-		auto contractType = make_shared<ContractType>(*contract);
-		TypePointers parameterTypes = contractType->constructorType()->parameterTypes();
-		_newExpression.annotation().type = make_shared<FunctionType>(
-			parameterTypes,
-			TypePointers{contractType},
-			strings(),
-			strings(),
-			FunctionType::Location::Creation
-		);
+		_newExpression.annotation().type = FunctionType::newExpressionType(*contract);
 	}
 	else if (type->category() == Type::Category::Array)
 	{
diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp
index ca8490f2..a0c1626d 100644
--- a/libsolidity/ast/Types.cpp
+++ b/libsolidity/ast/Types.cpp
@@ -362,8 +362,8 @@ MemberList::MemberMap IntegerType::nativeMembers(ContractDefinition const*) cons
 	if (isAddress())
 		return {
 			{"balance", make_shared<IntegerType >(256)},
-			{"call", make_shared<FunctionType>(strings(), strings{"bool"}, FunctionType::Location::Bare, true)},
-			{"callcode", make_shared<FunctionType>(strings(), strings{"bool"}, FunctionType::Location::BareCallCode, true)},
+			{"call", make_shared<FunctionType>(strings(), strings{"bool"}, FunctionType::Location::Bare, true, false, true)},
+			{"callcode", make_shared<FunctionType>(strings(), strings{"bool"}, FunctionType::Location::BareCallCode, true, false, true)},
 			{"delegatecall", make_shared<FunctionType>(strings(), strings{"bool"}, FunctionType::Location::BareDelegateCall, true)},
 			{"send", make_shared<FunctionType>(strings{"uint"}, strings{"bool"}, FunctionType::Location::Send)}
 		};
@@ -1329,16 +1329,10 @@ MemberList::MemberMap ContractType::nativeMembers(ContractDefinition const*) con
 	return members;
 }
 
-shared_ptr<FunctionType const> const& ContractType::constructorType() const
+shared_ptr<FunctionType const> const& ContractType::newExpressionType() const
 {
 	if (!m_constructorType)
-	{
-		FunctionDefinition const* constructor = m_contract.constructor();
-		if (constructor)
-			m_constructorType = make_shared<FunctionType>(*constructor);
-		else
-			m_constructorType = make_shared<FunctionType>(TypePointers(), TypePointers());
-	}
+		m_constructorType = FunctionType::newExpressionType(m_contract);
 	return m_constructorType;
 }
 
@@ -1738,7 +1732,7 @@ FunctionType::FunctionType(VariableDeclaration const& _varDecl):
 	swap(retParamNames, m_returnParameterNames);
 }
 
-FunctionType::FunctionType(const EventDefinition& _event):
+FunctionType::FunctionType(EventDefinition const& _event):
 	m_location(Location::Event), m_isConstant(true), m_declaration(&_event)
 {
 	TypePointers params;
@@ -1754,6 +1748,35 @@ FunctionType::FunctionType(const EventDefinition& _event):
 	swap(paramNames, m_parameterNames);
 }
 
+FunctionTypePointer FunctionType::newExpressionType(ContractDefinition const& _contract)
+{
+	FunctionDefinition const* constructor = _contract.constructor();
+	TypePointers parameters;
+	strings parameterNames;
+	bool payable = false;
+
+	if (constructor)
+	{
+		for (ASTPointer<VariableDeclaration> const& var: constructor->parameters())
+		{
+			parameterNames.push_back(var->name());
+			parameters.push_back(var->annotation().type);
+		}
+		payable = constructor->isPayable();
+	}
+	return make_shared<FunctionType>(
+		parameters,
+		TypePointers{make_shared<ContractType>(_contract)},
+		parameterNames,
+		strings{""},
+		Location::Creation,
+		false,
+		nullptr,
+		false,
+		payable
+	);
+}
+
 vector<string> FunctionType::parameterNames() const
 {
 	if (!bound())
@@ -1872,7 +1895,12 @@ FunctionTypePointer FunctionType::interfaceFunctionType() const
 	if (variable && retParamTypes.empty())
 		return FunctionTypePointer();
 
-	return make_shared<FunctionType>(paramTypes, retParamTypes, m_parameterNames, m_returnParameterNames, m_location, m_arbitraryParameters);
+	return make_shared<FunctionType>(
+		paramTypes, retParamTypes,
+		m_parameterNames, m_returnParameterNames,
+		m_location, m_arbitraryParameters,
+		m_declaration, m_isConstant, m_isPayable
+	);
 }
 
 MemberList::MemberMap FunctionType::nativeMembers(ContractDefinition const*) const
@@ -1891,7 +1919,7 @@ MemberList::MemberMap FunctionType::nativeMembers(ContractDefinition const*) con
 		MemberList::MemberMap members;
 		if (m_location != Location::BareDelegateCall && m_location != Location::DelegateCall)
 		{
-			if (!m_declaration || m_isPayable) // If this is an actual function, it has to be payable
+			if (m_isPayable)
 				members.push_back(MemberList::Member(
 					"value",
 					make_shared<FunctionType>(
@@ -1902,6 +1930,8 @@ MemberList::MemberMap FunctionType::nativeMembers(ContractDefinition const*) con
 						Location::SetValue,
 						false,
 						nullptr,
+						false,
+						false,
 						m_gasSet,
 						m_valueSet
 					)
@@ -1918,6 +1948,8 @@ MemberList::MemberMap FunctionType::nativeMembers(ContractDefinition const*) con
 					Location::SetGas,
 					false,
 					nullptr,
+					false,
+					false,
 					m_gasSet,
 					m_valueSet
 				)
@@ -2023,6 +2055,8 @@ TypePointer FunctionType::copyAndSetGasOrValue(bool _setGas, bool _setValue) con
 		m_location,
 		m_arbitraryParameters,
 		m_declaration,
+		m_isConstant,
+		m_isPayable,
 		m_gasSet || _setGas,
 		m_valueSet || _setValue,
 		m_bound
@@ -2068,6 +2102,8 @@ FunctionTypePointer FunctionType::asMemberFunction(bool _inLibrary, bool _bound)
 		location,
 		m_arbitraryParameters,
 		m_declaration,
+		m_isConstant,
+		m_isPayable,
 		m_gasSet,
 		m_valueSet,
 		_bound
diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h
index c57f8db1..9173f39a 100644
--- a/libsolidity/ast/Types.h
+++ b/libsolidity/ast/Types.h
@@ -640,9 +640,8 @@ public:
 	bool isSuper() const { return m_super; }
 	ContractDefinition const& contractDefinition() const { return m_contract; }
 
-	/// Returns the function type of the constructor. Note that the location part of the function type
-	/// is not used, as this type cannot be the type of a variable or expression.
-	FunctionTypePointer const& constructorType() const;
+	/// Returns the function type of the constructor modified to return an object of the contract's type.
+	FunctionTypePointer const& newExpressionType() const;
 
 	/// @returns the identifier of the function with the given name or Invalid256 if such a name does
 	/// not exist.
@@ -820,21 +819,32 @@ public:
 	explicit FunctionType(VariableDeclaration const& _varDecl);
 	/// Creates the function type of an event.
 	explicit FunctionType(EventDefinition const& _event);
+	/// Function type constructor to be used for a plain type (not derived from a declaration).
 	FunctionType(
 		strings const& _parameterTypes,
 		strings const& _returnParameterTypes,
 		Location _location = Location::Internal,
-		bool _arbitraryParameters = false
+		bool _arbitraryParameters = false,
+		bool _constant = false,
+		bool _payable = false
 	): FunctionType(
 		parseElementaryTypeVector(_parameterTypes),
 		parseElementaryTypeVector(_returnParameterTypes),
 		strings(),
 		strings(),
 		_location,
-		_arbitraryParameters
+		_arbitraryParameters,
+		nullptr,
+		_constant,
+		_payable
 	)
 	{
 	}
+
+	/// @returns the type of the "new Contract" function, i.e. basically the constructor.
+	static FunctionTypePointer newExpressionType(ContractDefinition const& _contract);
+
+	/// Detailed constructor, use with care.
 	FunctionType(
 		TypePointers const& _parameterTypes,
 		TypePointers const& _returnParameterTypes,
@@ -843,6 +853,8 @@ public:
 		Location _location = Location::Internal,
 		bool _arbitraryParameters = false,
 		Declaration const* _declaration = nullptr,
+		bool _isConstant = false,
+		bool _isPayable = false,
 		bool _gasSet = false,
 		bool _valueSet = false,
 		bool _bound = false
@@ -856,6 +868,8 @@ public:
 		m_gasSet(_gasSet),
 		m_valueSet(_valueSet),
 		m_bound(_bound),
+		m_isConstant(_isConstant),
+		m_isPayable(_isPayable),
 		m_declaration(_declaration)
 	{}
 
diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp
index 58e6847c..33571bc0 100644
--- a/libsolidity/codegen/ContractCompiler.cpp
+++ b/libsolidity/codegen/ContractCompiler.cpp
@@ -242,6 +242,12 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac
 	m_context << notFound;
 	if (fallback)
 	{
+		if (!fallback->isPayable())
+		{
+			// Throw if function is not payable but call contained ether.
+			m_context << Instruction::CALLVALUE;
+			m_context.appendConditionalJumpTo(m_context.errorTag());
+		}
 		eth::AssemblyItem returnTag = m_context.pushNewTag();
 		fallback->accept(*this);
 		m_context << returnTag;
diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index 4a81e27d..de937b42 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -583,6 +583,8 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 					Location::Bare,
 					false,
 					nullptr,
+					false,
+					false,
 					true,
 					true
 				),
diff --git a/test/libsolidity/Assembly.cpp b/test/libsolidity/Assembly.cpp
index 81332f4f..8d7a3540 100644
--- a/test/libsolidity/Assembly.cpp
+++ b/test/libsolidity/Assembly.cpp
@@ -115,7 +115,7 @@ BOOST_AUTO_TEST_CASE(location_test)
 	AssemblyItems items = compileContract(sourceCode);
 	vector<SourceLocation> locations =
 		vector<SourceLocation>(18, SourceLocation(2, 75, n)) +
-		vector<SourceLocation>(28, SourceLocation(20, 72, n)) +
+		vector<SourceLocation>(31, SourceLocation(20, 72, n)) +
 		vector<SourceLocation>{SourceLocation(42, 51, n), SourceLocation(65, 67, n)} +
 		vector<SourceLocation>(4, SourceLocation(58, 67, n)) +
 		vector<SourceLocation>(3, SourceLocation(20, 72, n));
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index c1f1b148..006d41c2 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -1526,8 +1526,10 @@ BOOST_AUTO_TEST_CASE(convert_uint_to_fixed_bytes_greater_size)
 			}
 		})";
 	compileAndRun(sourceCode);
-	BOOST_CHECK(callContractFunction("UintToBytes(uint16)", u256("0x6162")) ==
-                encodeArgs(string("\0\0\0\0\0\0ab", 8)));
+	BOOST_CHECK(
+		callContractFunction("UintToBytes(uint16)", u256("0x6162")) ==
+		encodeArgs(string("\0\0\0\0\0\0ab", 8))
+	);
 }
 
 BOOST_AUTO_TEST_CASE(send_ether)
@@ -2053,7 +2055,7 @@ BOOST_AUTO_TEST_CASE(contracts_as_addresses)
 {
 	char const* sourceCode = R"(
 		contract helper {
-			function() { } // can receive ether
+			function() payable { } // can receive ether
 		}
 		contract test {
 			helper h;
@@ -2065,6 +2067,7 @@ BOOST_AUTO_TEST_CASE(contracts_as_addresses)
 		}
 	)";
 	compileAndRun(sourceCode, 20);
+	BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 20 - 5);
 	BOOST_REQUIRE(callContractFunction("getBalance()") == encodeArgs(u256(20 - 5), u256(5)));
 }
 
@@ -2938,11 +2941,10 @@ BOOST_AUTO_TEST_CASE(generic_delegatecall)
 				uint public received;
 				address public sender;
 				uint public value;
-				function sender() payable {}
 				function doSend(address rec) payable
 				{
 					bytes4 signature = bytes4(bytes32(sha3("receive(uint256)")));
-					rec.delegatecall(signature, 23);
+					if (rec.delegatecall(signature, 23)) {}
 				}
 			}
 	)**";
@@ -5961,7 +5963,7 @@ BOOST_AUTO_TEST_CASE(reject_ether_sent_to_library)
 			function f(address x) returns (bool) {
 				return x.send(1);
 			}
-			function () {}
+			function () payable {}
 		}
 	)";
 	compileAndRun(sourceCode, 0, "lib");
@@ -6879,7 +6881,7 @@ BOOST_AUTO_TEST_CASE(skip_dynamic_types_for_structs)
 BOOST_AUTO_TEST_CASE(failed_create)
 {
 	char const* sourceCode = R"(
-		contract D { }
+		contract D { function D() payable {} }
 		contract C {
 			uint public x;
 			function f(uint amount) returns (address) {
@@ -7029,7 +7031,7 @@ BOOST_AUTO_TEST_CASE(mutex)
 				else
 					return fund.withdrawUnprotected(10);
 			}
-			function() {
+			function() payable {
 				callDepth++;
 				if (callDepth < 4)
 					attackInternal();
@@ -7104,55 +7106,47 @@ BOOST_AUTO_TEST_CASE(payable_function)
 {
 	char const* sourceCode = R"(
 		contract C {
+			uint public a;
 			function f() payable returns (uint) {
 				return msg.value;
 			}
-			function() payable returns (uint) {
-				return msg.value;
+			function() payable {
+				a = msg.value + 1;
 			}
 		}
 	)";
 	compileAndRun(sourceCode, 0, "C");
 	BOOST_CHECK(callContractFunctionWithValue("f()", 27) == encodeArgs(u256(27)));
-	BOOST_CHECK(callContractFunctionWithValue("", 27) == encodeArgs(u256(27)));
-}
-
-BOOST_AUTO_TEST_CASE(non_payable_throw_constructor)
-{
-	char const* sourceCode = R"(
-		contract C {
-			function C() { }
-		}
-		contract D {
-			function D() payable {}
-			function f() returns (uint) {
-				(new C).value(2)();
-				return 2;
-			}
-		}
-	)";
-	compileAndRun(sourceCode, 27, "D");
-	BOOST_CHECK(callContractFunction("f()") == encodeArgs());
 	BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 27);
+	BOOST_CHECK(callContractFunctionWithValue("", 27) == encodeArgs());
+	BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 27 + 27);
+	BOOST_CHECK(callContractFunction("a()") == encodeArgs(u256(28)));
+	BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 27 + 27);
 }
 
 BOOST_AUTO_TEST_CASE(non_payable_throw)
 {
 	char const* sourceCode = R"(
 		contract C {
-			string public a;
+			uint public a;
 			function f() returns (uint) {
 				return msg.value;
 			}
-			function() returns (uint) {
-				return msg.value;
+			function() {
+				a = msg.value + 1;
 			}
 		}
 	)";
 	compileAndRun(sourceCode, 0, "C");
 	BOOST_CHECK(callContractFunctionWithValue("f()", 27) == encodeArgs());
+	BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 0);
+	BOOST_CHECK(callContractFunction("") == encodeArgs());
+	BOOST_CHECK(callContractFunction("a()") == encodeArgs(u256(1)));
 	BOOST_CHECK(callContractFunctionWithValue("", 27) == encodeArgs());
+	BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 0);
+	BOOST_CHECK(callContractFunction("a()") == encodeArgs(u256(1)));
 	BOOST_CHECK(callContractFunctionWithValue("a()", 27) == encodeArgs());
+	BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 0);
 }
 
 BOOST_AUTO_TEST_CASE(no_nonpayable_circumvention_by_modifier)
@@ -7169,6 +7163,7 @@ BOOST_AUTO_TEST_CASE(no_nonpayable_circumvention_by_modifier)
 	)";
 	compileAndRun(sourceCode);
 	BOOST_CHECK(callContractFunctionWithValue("f()", 27) == encodeArgs());
+	BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 0);
 }
 
 
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index e193745d..3adf4612 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -3911,7 +3911,23 @@ BOOST_AUTO_TEST_CASE(calling_nonpayable)
 	char const* text = R"(
 		contract receiver { function nopay() {} }
 		contract test {
-			function_argument_mem_to_storage f() { (new receiver()).nopay.value(10)(); }
+			function f() { (new receiver()).nopay.value(10)(); }
+		}
+	)";
+	BOOST_CHECK(expectError(text) == Error::Type::TypeError);
+}
+
+BOOST_AUTO_TEST_CASE(non_payable_constructor)
+{
+	char const* text = R"(
+		contract C {
+			function C() { }
+		}
+		contract D {
+			function f() returns (uint) {
+				(new C).value(2)();
+				return 2;
+			}
 		}
 	)";
 	BOOST_CHECK(expectError(text) == Error::Type::TypeError);