Merge pull request #2698 from ethereum/unimplemented-constructor

Constructors must be implemented if declared.

4 files changed, 15 insertions, 47 deletions

diff --git a/Changelog.md b/Changelog.md
index f7eb8070..8590f7b9 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -6,6 +6,7 @@ Features:
 Bugfixes:
  * Code Generator: ``.delegatecall()`` should always return execution outcome.
  * Code Generator: Provide "new account gas" for low-level ``callcode`` and ``delegatecall``.
+ * Type Checker: Constructors must be implemented if declared.
  * Type Checker: Do not mark overloaded functions as shadowing other functions.
  * Type Checker: Disallow the ``.gas()`` modifier on ``ecrecover``, ``sha256`` and ``ripemd160``.
 
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index ce4baa3f..a9f5b931 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -192,13 +192,7 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont
 		{
 			// Take constructors out of overload hierarchy
 			if (function->isConstructor())
-			{
-				if (!function->isImplemented())
-					// Base contract's constructor is not fully implemented, no way to get
-					// out of this.
-					_contract.annotation().unimplementedFunctions.push_back(function);
 				continue;
-			}
 			auto& overloads = functions[function->name()];
 			FunctionTypePointer funType = make_shared<FunctionType>(*function);
 			auto it = find_if(overloads.begin(), overloads.end(), [&](FunTypeAndFlag const& _funAndFlag)
@@ -522,6 +516,8 @@ bool TypeChecker::visit(FunctionDefinition const& _function)
 	}
 	if (_function.isImplemented())
 		_function.body().accept(*this);
+	else if (_function.isConstructor())
+		m_errorReporter.typeError(_function.location(), "Constructor must be implemented if declared.");
 	return false;
 }
 
diff --git a/libsolidity/ast/ASTAnnotations.h b/libsolidity/ast/ASTAnnotations.h
index ecddbe21..f757f03c 100644
--- a/libsolidity/ast/ASTAnnotations.h
+++ b/libsolidity/ast/ASTAnnotations.h
@@ -79,8 +79,7 @@ struct TypeDeclarationAnnotation: ASTAnnotation
 
 struct ContractDefinitionAnnotation: TypeDeclarationAnnotation, DocumentedAnnotation
 {
-	/// List of functions without a body. Can also contain functions from base classes,
-	/// especially constructors.
+	/// List of functions without a body. Can also contain functions from base classes.
 	std::vector<FunctionDefinition const*> unimplementedFunctions;
 	/// List of all (direct and indirect) base contracts in order from derived to
 	/// base, including the contract itself.
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index 37bbc2eb..aaf1a449 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -677,44 +677,6 @@ BOOST_AUTO_TEST_CASE(create_abstract_contract)
 	CHECK_ERROR(text, TypeError, "");
 }
 
-BOOST_AUTO_TEST_CASE(abstract_contract_constructor_args_optional)
-{
-	ASTPointer<SourceUnit> sourceUnit;
-	char const* text = R"(
-		contract BaseBase { function BaseBase(uint j); }
-		contract base is BaseBase { function foo(); }
-		contract derived is base {
-			function derived(uint i) BaseBase(i){}
-			function foo() {}
-		}
-	)";
-	ETH_TEST_REQUIRE_NO_THROW(sourceUnit = parseAndAnalyse(text), "Parsing and name resolving failed");
-	std::vector<ASTPointer<ASTNode>> nodes = sourceUnit->nodes();
-	BOOST_CHECK_EQUAL(nodes.size(), 4);
-	ContractDefinition* derived = dynamic_cast<ContractDefinition*>(nodes[3].get());
-	BOOST_REQUIRE(derived);
-	BOOST_CHECK(!derived->annotation().unimplementedFunctions.empty());
-}
-
-BOOST_AUTO_TEST_CASE(abstract_contract_constructor_args_not_provided)
-{
-	ASTPointer<SourceUnit> sourceUnit;
-	char const* text = R"(
-		contract BaseBase { function BaseBase(uint); }
-		contract base is BaseBase { function foo(); }
-		contract derived is base {
-			function derived(uint) {}
-			function foo() {}
-		}
-	)";
-	ETH_TEST_REQUIRE_NO_THROW(sourceUnit = parseAndAnalyse(text), "Parsing and name resolving failed");
-	std::vector<ASTPointer<ASTNode>> nodes = sourceUnit->nodes();
-	BOOST_CHECK_EQUAL(nodes.size(), 4);
-	ContractDefinition* derived = dynamic_cast<ContractDefinition*>(nodes[3].get());
-	BOOST_REQUIRE(derived);
-	BOOST_CHECK(!derived->annotation().unimplementedFunctions.empty());
-}
-
 BOOST_AUTO_TEST_CASE(redeclare_implemented_abstract_function_as_abstract)
 {
 	ASTPointer<SourceUnit> sourceUnit;
@@ -5714,7 +5676,7 @@ BOOST_AUTO_TEST_CASE(interface_constructor)
 			function I();
 		}
 	)";
-	CHECK_ERROR(text, TypeError, "Constructor cannot be defined in interfaces");
+	CHECK_ERROR_ALLOW_MULTI(text, TypeError, "Constructor cannot be defined in interfaces");
 }
 
 BOOST_AUTO_TEST_CASE(interface_functions)
@@ -6564,6 +6526,16 @@ BOOST_AUTO_TEST_CASE(builtin_reject_value)
 	CHECK_ERROR(text, TypeError, "Member \"value\" not found or not visible after argument-dependent lookup");
 }
 
+BOOST_AUTO_TEST_CASE(constructor_without_implementation)
+{
+	char const* text = R"(
+		contract C {
+			function C();
+		}
+	)";
+	CHECK_ERROR(text, TypeError, "Constructor must be implemented if declared.");
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }