Merge pull request #3824 from ethereum/baseArgumentsEmptyParenthesis

Error when using empty parentheses for base class constructors that r…

10 files changed, 73 insertions, 36 deletions

diff --git a/Changelog.md b/Changelog.md
index 7e653ebf..d6860bdf 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -11,6 +11,7 @@ Features:
  * Optimizer: Optimize across ``mload`` if ``msize()`` is not used.
  * Syntax Checker: Issue warning for empty structs (or error as experimental 0.5.0 feature).
  * General: Introduce new constructor syntax using the ``constructor`` keyword as experimental 0.5.0 feature.
+ * Inheritance: Error when using empty parenthesis for base class constructors that require arguments as experimental 0.5.0 feature.
 
 Bugfixes:
  * Code Generator: Allow ``block.blockhash`` without being called.
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index 620dfca4..a252742d 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -320,7 +320,7 @@ void TypeChecker::checkContractAbstractConstructors(ContractDefinition const& _c
 		{
 			auto baseContract = dynamic_cast<ContractDefinition const*>(&dereference(base->name()));
 			solAssert(baseContract, "");
-			if (!base->arguments().empty())
+			if (base->arguments() && !base->arguments()->empty())
 				argumentsNeeded.erase(baseContract);
 		}
 	}
@@ -506,30 +506,46 @@ void TypeChecker::endVisit(InheritanceSpecifier const& _inheritance)
 		// Interfaces do not have constructors, so there are zero parameters.
 		parameterTypes = ContractType(*base).newExpressionType()->parameterTypes();
 
-	if (!arguments.empty() && parameterTypes.size() != arguments.size())
+	if (arguments)
 	{
-		m_errorReporter.typeError(
-			_inheritance.location(),
-			"Wrong argument count for constructor call: " +
-			toString(arguments.size()) +
-			" arguments given but expected " +
-			toString(parameterTypes.size()) +
-			"."
-		);
-		return;
-	}
+		bool v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050);
 
-	for (size_t i = 0; i < arguments.size(); ++i)
-		if (!type(*arguments[i])->isImplicitlyConvertibleTo(*parameterTypes[i]))
-			m_errorReporter.typeError(
-				arguments[i]->location(),
-				"Invalid type for argument in constructor call. "
-				"Invalid implicit conversion from " +
-				type(*arguments[i])->toString() +
-				" to " +
-				parameterTypes[i]->toString() +
-				" requested."
-			);
+		if (parameterTypes.size() != arguments->size())
+		{
+			if (arguments->size() == 0 && !v050)
+				m_errorReporter.warning(
+					_inheritance.location(),
+					"Wrong argument count for constructor call: " +
+					toString(arguments->size()) +
+					" arguments given but expected " +
+					toString(parameterTypes.size()) +
+					"."
+				);
+			else
+			{
+				m_errorReporter.typeError(
+					_inheritance.location(),
+					"Wrong argument count for constructor call: " +
+					toString(arguments->size()) +
+					" arguments given but expected " +
+					toString(parameterTypes.size()) +
+					"."
+				);
+				return;
+			}
+		}
+		for (size_t i = 0; i < arguments->size(); ++i)
+			if (!type(*(*arguments)[i])->isImplicitlyConvertibleTo(*parameterTypes[i]))
+				m_errorReporter.typeError(
+					(*arguments)[i]->location(),
+					"Invalid type for argument in constructor call. "
+					"Invalid implicit conversion from " +
+					type(*(*arguments)[i])->toString() +
+					" to " +
+					parameterTypes[i]->toString() +
+					" requested."
+				);
+	}
 }
 
 void TypeChecker::endVisit(UsingForDirective const& _usingFor)
diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h
index 56bb412c..bc85349b 100644
--- a/libsolidity/ast/AST.h
+++ b/libsolidity/ast/AST.h
@@ -425,19 +425,22 @@ public:
 	InheritanceSpecifier(
 		SourceLocation const& _location,
 		ASTPointer<UserDefinedTypeName> const& _baseName,
-		std::vector<ASTPointer<Expression>> _arguments
+		std::unique_ptr<std::vector<ASTPointer<Expression>>> _arguments
 	):
-		ASTNode(_location), m_baseName(_baseName), m_arguments(_arguments) {}
+		ASTNode(_location), m_baseName(_baseName), m_arguments(std::move(_arguments)) {}
 
 	virtual void accept(ASTVisitor& _visitor) override;
 	virtual void accept(ASTConstVisitor& _visitor) const override;
 
 	UserDefinedTypeName const& name() const { return *m_baseName; }
-	std::vector<ASTPointer<Expression>> const& arguments() const { return m_arguments; }
+	// Returns nullptr if no argument list was given (``C``).
+	// If an argument list is given (``C(...)``), the arguments are returned
+	// as a vector of expressions. Note that this vector can be empty (``C()``).
+	std::vector<ASTPointer<Expression>> const* arguments() const { return m_arguments.get(); }
 
 private:
 	ASTPointer<UserDefinedTypeName> m_baseName;
-	std::vector<ASTPointer<Expression>> m_arguments;
+	std::unique_ptr<std::vector<ASTPointer<Expression>>> m_arguments;
 };
 
 /**
diff --git a/libsolidity/ast/ASTJsonConverter.cpp b/libsolidity/ast/ASTJsonConverter.cpp
index 4fef67c3..94932eca 100644
--- a/libsolidity/ast/ASTJsonConverter.cpp
+++ b/libsolidity/ast/ASTJsonConverter.cpp
@@ -268,7 +268,7 @@ bool ASTJsonConverter::visit(InheritanceSpecifier const& _node)
 {
 	setJsonNode(_node, "InheritanceSpecifier", {
 		make_pair("baseName", toJson(_node.name())),
-		make_pair("arguments", toJson(_node.arguments()))
+		make_pair("arguments", _node.arguments() ? toJson(*_node.arguments()) : Json::Value(Json::arrayValue))
 	});
 	return false;
 }
diff --git a/libsolidity/ast/AST_accept.h b/libsolidity/ast/AST_accept.h
index 70ee997e..dac414fc 100644
--- a/libsolidity/ast/AST_accept.h
+++ b/libsolidity/ast/AST_accept.h
@@ -94,7 +94,8 @@ void InheritanceSpecifier::accept(ASTVisitor& _visitor)
 	if (_visitor.visit(*this))
 	{
 		m_baseName->accept(_visitor);
-		listAccept(m_arguments, _visitor);
+		if (m_arguments)
+			listAccept(*m_arguments, _visitor);
 	}
 	_visitor.endVisit(*this);
 }
@@ -104,7 +105,8 @@ void InheritanceSpecifier::accept(ASTConstVisitor& _visitor) const
 	if (_visitor.visit(*this))
 	{
 		m_baseName->accept(_visitor);
-		listAccept(m_arguments, _visitor);
+		if (m_arguments)
+			listAccept(*m_arguments, _visitor);
 	}
 	_visitor.endVisit(*this);
 }
diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp
index ebd9139a..d3a7e4ea 100644
--- a/libsolidity/codegen/ContractCompiler.cpp
+++ b/libsolidity/codegen/ContractCompiler.cpp
@@ -157,8 +157,8 @@ void ContractCompiler::appendInitAndConstructorCode(ContractDefinition const& _c
 			);
 			solAssert(baseContract, "");
 
-			if (!m_baseArguments.count(baseContract->constructor()) && !base->arguments().empty())
-				m_baseArguments[baseContract->constructor()] = &base->arguments();
+			if (!m_baseArguments.count(baseContract->constructor()) && base->arguments() && !base->arguments()->empty())
+				m_baseArguments[baseContract->constructor()] = base->arguments();
 		}
 	}
 	// Initialization of state variables in base-to-derived order.
diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp
index 3dbd4c8f..9a7731d8 100644
--- a/libsolidity/parsing/Parser.cpp
+++ b/libsolidity/parsing/Parser.cpp
@@ -286,17 +286,17 @@ ASTPointer<InheritanceSpecifier> Parser::parseInheritanceSpecifier()
 	RecursionGuard recursionGuard(*this);
 	ASTNodeFactory nodeFactory(*this);
 	ASTPointer<UserDefinedTypeName> name(parseUserDefinedTypeName());
-	vector<ASTPointer<Expression>> arguments;
+	unique_ptr<vector<ASTPointer<Expression>>> arguments;
 	if (m_scanner->currentToken() == Token::LParen)
 	{
 		m_scanner->next();
-		arguments = parseFunctionCallListArguments();
+		arguments.reset(new vector<ASTPointer<Expression>>(parseFunctionCallListArguments()));
 		nodeFactory.markEndPosition();
 		expectToken(Token::RParen);
 	}
 	else
 		nodeFactory.setEndPositionFromNode(name);
-	return nodeFactory.createNode<InheritanceSpecifier>(name, arguments);
+	return nodeFactory.createNode<InheritanceSpecifier>(name, std::move(arguments));
 }
 
 Declaration::Visibility Parser::parseVisibilitySpecifier(Token::Value _token)
diff --git a/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses.sol b/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses.sol
index 9607ed60..b3fbd04a 100644
--- a/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses.sol
+++ b/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses.sol
@@ -3,4 +3,5 @@ contract Base {
 }
 contract Derived is Base(2) { }
 contract Derived2 is Base(), Derived() { }
-contract Derived3 is Base, Derived {}
+// ----
+// Warning: Wrong argument count for constructor call: 0 arguments given but expected 1.
diff --git a/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses_V050.sol b/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses_V050.sol
new file mode 100644
index 00000000..b3728634
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses_V050.sol
@@ -0,0 +1,9 @@
+pragma experimental "v0.5.0";
+
+contract Base {
+  constructor(uint) public {}
+}
+contract Derived is Base(2) { }
+contract Derived2 is Base(), Derived() { }
+// ----
+// TypeError: Wrong argument count for constructor call: 0 arguments given but expected 1.
diff --git a/test/libsolidity/syntaxTests/inheritance/base_arguments_no_parentheses.sol b/test/libsolidity/syntaxTests/inheritance/base_arguments_no_parentheses.sol
new file mode 100644
index 00000000..24cca8f0
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inheritance/base_arguments_no_parentheses.sol
@@ -0,0 +1,5 @@
+contract Base {
+  constructor(uint) public {}
+}
+contract Derived is Base(2) { }
+contract Derived2 is Base, Derived {}