Fix name clashes between constructor and fallback function.

Changelog.md

@@ -4,7 +4,8 @@ Features:
  * SMTChecker: Integration with CVC4 SMT solver
 
 Bugfixes:
-
+ * Type Checker: Do not complain about new-style constructor and fallback function to have the same name.
+ * Type Checker: Detect multiple constructor declarations in the new syntax and old syntax.
 
 ### 0.4.22 (2018-04-16)
 

libsolidity/analysis/TypeChecker.cpp

@@ -109,39 +109,28 @@ bool TypeChecker::visit(ContractDefinition const& _contract)
 			m_errorReporter.typeError(function->location(), "Constructor must be public or internal.");
 	}
 
-	FunctionDefinition const* fallbackFunction = nullptr;
 	for (FunctionDefinition const* function: _contract.definedFunctions())
-	{
 		if (function->isFallback())
 		{
-			if (fallbackFunction)
+			if (_contract.isLibrary())
-			{
+				m_errorReporter.typeError(function->location(), "Libraries cannot have fallback functions.");
-				m_errorReporter.declarationError(function->location(), "Only one fallback function is allowed.");
+			if (function->stateMutability() != StateMutability::NonPayable && function->stateMutability() != StateMutability::Payable)
-			}
+				m_errorReporter.typeError(
-			else
+					function->location(),
-			{
+					"Fallback function must be payable or non-payable, but is \"" +
-				fallbackFunction = function;
+					stateMutabilityToString(function->stateMutability()) +
-				if (_contract.isLibrary())
+					"\"."
-					m_errorReporter.typeError(fallbackFunction->location(), "Libraries cannot have fallback functions.");
+			);
-				if (function->stateMutability() != StateMutability::NonPayable && function->stateMutability() != StateMutability::Payable)
+			if (!function->parameters().empty())
-					m_errorReporter.typeError(
+				m_errorReporter.typeError(function->parameterList().location(), "Fallback function cannot take parameters.");
-						function->location(),
+			if (!function->returnParameters().empty())
-						"Fallback function must be payable or non-payable, but is \"" +
+				m_errorReporter.typeError(function->returnParameterList()->location(), "Fallback function cannot return values.");
-						stateMutabilityToString(function->stateMutability()) +
+			if (
-						"\"."
+				_contract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050) &&
-				);
+				function->visibility() != FunctionDefinition::Visibility::External
-				if (!fallbackFunction->parameters().empty())
+			)
-					m_errorReporter.typeError(fallbackFunction->parameterList().location(), "Fallback function cannot take parameters.");
+				m_errorReporter.typeError(function->location(), "Fallback function must be defined as \"external\".");
-				if (!fallbackFunction->returnParameters().empty())
-					m_errorReporter.typeError(fallbackFunction->returnParameterList()->location(), "Fallback function cannot return values.");
-				if (
-					_contract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050) &&
-					fallbackFunction->visibility() != FunctionDefinition::Visibility::External
-				)
-					m_errorReporter.typeError(fallbackFunction->location(), "Fallback function must be defined as \"external\".");
-			}
 		}
-	}
 
 	for (auto const& n: _contract.subNodes())
 		if (!visited.count(n.get()))
@@ -172,25 +161,34 @@ void TypeChecker::checkContractDuplicateFunctions(ContractDefinition const& _con
 	/// Checks that two functions with the same name defined in this contract have different
 	/// argument types and that there is at most one constructor.
 	map<string, vector<FunctionDefinition const*>> functions;
+	FunctionDefinition const* constructor = nullptr;
+	FunctionDefinition const* fallback = nullptr;
 	for (FunctionDefinition const* function: _contract.definedFunctions())
-		functions[function->name()].push_back(function);
+		if (function->isConstructor())
-
+		{
-	// Constructor
+			if (constructor)
-	if (functions[_contract.name()].size() > 1)
+				m_errorReporter.declarationError(
-	{
+					function->location(),
-		SecondarySourceLocation ssl;
+					SecondarySourceLocation().append("Another declaration is here:", constructor->location()),
-		auto it = ++functions[_contract.name()].begin();
+					"More than one constructor defined."
-		for (; it != functions[_contract.name()].end(); ++it)
+				);
-			ssl.append("Another declaration is here:", (*it)->location());
+			constructor = function;
-
+		}
-		string msg = "More than one constructor defined.";
+		else if (function->isFallback())
-		ssl.limitSize(msg);
+		{
-		m_errorReporter.declarationError(
+			if (fallback)
-			functions[_contract.name()].front()->location(),
+				m_errorReporter.declarationError(
-			ssl,
+					function->location(),
-			msg
+					SecondarySourceLocation().append("Another declaration is here:", fallback->location()),
-		);
+					"Only one fallback function is allowed."
-	}
+				);
+			fallback = function;
+		}
+		else
+		{
+			solAssert(!function->name().empty(), "");
+			functions[function->name()].push_back(function);
+		}
 
 	findDuplicateDefinitions(functions, "Function with same name and arguments defined twice.");
 }

test/libsolidity/SolidityNameAndTypeResolution.cpp

@@ -1140,7 +1140,6 @@ BOOST_AUTO_TEST_CASE(fallback_function_twice)
 		}
 	)";
 	CHECK_ERROR_ALLOW_MULTI(text, DeclarationError, (vector<string>{
-		"Function with same name and arguments defined twice.",
 		"Only one fallback function is"
 	}));
 }

test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol

@@ -0,0 +1,7 @@
+contract test {
+	function test(uint) public { }
+	constructor() public {}
+}
+// ----
+// Warning: (17-47): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
+// DeclarationError: (49-72): More than one constructor defined.

test/libsolidity/syntaxTests/constructor/two_constructors_new.sol

@@ -0,0 +1,6 @@
+contract test {
+	constructor(uint) public { }
+	constructor() public {}
+}
+// ----
+// DeclarationError: (47-70): More than one constructor defined.

test/libsolidity/syntaxTests/constructor/two_constructors_old.sol

@@ -5,4 +5,4 @@ contract test {
 // ----
 // Warning: (17-49): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
 // Warning: (51-76): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
-// DeclarationError: (17-49): More than one constructor defined.
+// DeclarationError: (51-76): More than one constructor defined.