5 files changed, 82 insertions, 47 deletions

diff --git a/libsolidity/analysis/PostTypeChecker.cpp b/libsolidity/analysis/PostTypeChecker.cpp
index fbc72e52..19d0b708 100644
--- a/libsolidity/analysis/PostTypeChecker.cpp
+++ b/libsolidity/analysis/PostTypeChecker.cpp
@@ -21,6 +21,8 @@
 #include <libsolidity/interface/ErrorReporter.h>
 #include <libsolidity/interface/Version.h>
 
+#include <libdevcore/Algorithms.h>
+
 #include <boost/range/adaptor/map.hpp>
 
 #include <memory>
@@ -47,7 +49,7 @@ void PostTypeChecker::endVisit(ContractDefinition const&)
 {
 	solAssert(!m_currentConstVariable, "");
 	for (auto declaration: m_constVariables)
-		if (auto identifier = findCycle(declaration))
+		if (auto identifier = findCycle(*declaration))
 			m_errorReporter.typeError(
 				declaration->location(),
 				"The value of the constant " + declaration->name() +
@@ -87,20 +89,24 @@ bool PostTypeChecker::visit(Identifier const& _identifier)
 	return true;
 }
 
-VariableDeclaration const* PostTypeChecker::findCycle(
-	VariableDeclaration const* _startingFrom,
-	set<VariableDeclaration const*> const& _seen
-)
+VariableDeclaration const* PostTypeChecker::findCycle(VariableDeclaration const& _startingFrom)
 {
-	if (_seen.count(_startingFrom))
-		return _startingFrom;
-	else if (m_constVariableDependencies.count(_startingFrom))
+	auto visitor = [&](VariableDeclaration const& _variable, CycleDetector<VariableDeclaration>& _cycleDetector)
 	{
-		set<VariableDeclaration const*> seen(_seen);
-		seen.insert(_startingFrom);
-		for (auto v: m_constVariableDependencies[_startingFrom])
-			if (findCycle(v, seen))
-				return v;
-	}
-	return nullptr;
+		// Iterating through the dependencies needs to be deterministic and thus cannot
+		// depend on the memory layout.
+		// Because of that, we sort by AST node id.
+		vector<VariableDeclaration const*> dependencies(
+			m_constVariableDependencies[&_variable].begin(),
+			m_constVariableDependencies[&_variable].end()
+		);
+		sort(dependencies.begin(), dependencies.end(), [](VariableDeclaration const* _a, VariableDeclaration const* _b) -> bool
+		{
+			return _a->id() < _b->id();
+		});
+		for (auto v: dependencies)
+			if (_cycleDetector.run(*v))
+				return;
+	};
+	return CycleDetector<VariableDeclaration>(visitor).run(_startingFrom);
 }
diff --git a/libsolidity/analysis/PostTypeChecker.h b/libsolidity/analysis/PostTypeChecker.h
index bafc1ae6..4f9dac6e 100644
--- a/libsolidity/analysis/PostTypeChecker.h
+++ b/libsolidity/analysis/PostTypeChecker.h
@@ -55,10 +55,7 @@ private:
 
 	virtual bool visit(Identifier const& _identifier) override;
 
-	VariableDeclaration const* findCycle(
-		VariableDeclaration const* _startingFrom,
-		std::set<VariableDeclaration const*> const& _seen = std::set<VariableDeclaration const*>{}
-	);
+	VariableDeclaration const* findCycle(VariableDeclaration const& _startingFrom);
 
 	ErrorReporter& m_errorReporter;
 
diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp
index 6aee260e..d96f8748 100644
--- a/libsolidity/analysis/StaticAnalyzer.cpp
+++ b/libsolidity/analysis/StaticAnalyzer.cpp
@@ -50,6 +50,16 @@ void StaticAnalyzer::endVisit(ContractDefinition const&)
 
 bool StaticAnalyzer::visit(FunctionDefinition const& _function)
 {
+	const bool isInterface = m_currentContract->contractKind() == ContractDefinition::ContractKind::Interface;
+
+	if (_function.noVisibilitySpecified())
+		m_errorReporter.warning(
+			_function.location(),
+			"No visibility specified. Defaulting to \"" +
+			Declaration::visibilityToString(_function.visibility()) +
+			"\". " +
+			(isInterface ? "In interfaces it defaults to external." : "")
+		);
 	if (_function.isImplemented())
 		m_currentFunction = &_function;
 	else
diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp
index 3a32810b..343b4ba8 100644
--- a/libsolidity/analysis/SyntaxChecker.cpp
+++ b/libsolidity/analysis/SyntaxChecker.cpp
@@ -214,16 +214,22 @@ bool SyntaxChecker::visit(FunctionDefinition const& _function)
 {
 	bool const v050 = m_sourceUnit->annotation().experimentalFeatures.count(ExperimentalFeature::V050);
 
-	if (_function.noVisibilitySpecified())
+	if (v050 && _function.noVisibilitySpecified())
+		m_errorReporter.syntaxError(_function.location(), "No visibility specified.");
+
+	if (_function.isOldStyleConstructor())
 	{
 		if (v050)
-			m_errorReporter.syntaxError(_function.location(), "No visibility specified.");
+			m_errorReporter.syntaxError(
+				_function.location(),
+				"Functions are not allowed to have the same name as the contract. "
+				"If you intend this to be a constructor, use \"constructor(...) { ... }\" to define it."
+			);
 		else
 			m_errorReporter.warning(
 				_function.location(),
-				"No visibility specified. Defaulting to \"" +
-				Declaration::visibilityToString(_function.visibility()) +
-				"\"."
+				"Defining constructors as functions with the same name as the contract is deprecated. "
+				"Use \"constructor(...) { ... }\" instead."
 			);
 	}
 	return true;
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)