Merge pull request #3892 from ethereum/develop

Merge develop into release for 0.4.22

4 files changed, 61 insertions, 29 deletions

diff --git a/libsolidity/parsing/DocStringParser.cpp b/libsolidity/parsing/DocStringParser.cpp
index 0409de72..d058d556 100644
--- a/libsolidity/parsing/DocStringParser.cpp
+++ b/libsolidity/parsing/DocStringParser.cpp
@@ -119,21 +119,17 @@ DocStringParser::iter DocStringParser::parseDocTagParam(iter _pos, iter _end)
 		return _end;
 	}
 	auto nameEndPos = firstSpaceOrTab(nameStartPos, _end);
-	if (nameEndPos == _end)
-	{
-		appendError("End of param name not found: " + string(nameStartPos, _end));
-		return _end;
-	}
 	auto paramName = string(nameStartPos, nameEndPos);
 
 	auto descStartPos = skipWhitespace(nameEndPos, _end);
-	if (descStartPos == _end)
+	auto nlPos = find(descStartPos, _end, '\n');
+
+	if (descStartPos == nlPos)
 	{
 		appendError("No description given for param " + paramName);
 		return _end;
 	}
 
-	auto nlPos = find(descStartPos, _end, '\n');
 	auto paramDesc = string(descStartPos, nlPos);
 	newTag("param");
 	m_lastTag->paramName = paramName;
diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp
index 8c97f55f..618a0896 100644
--- a/libsolidity/parsing/Parser.cpp
+++ b/libsolidity/parsing/Parser.cpp
@@ -238,7 +238,10 @@ ASTPointer<ContractDefinition> Parser::parseContractDefinition(Token::Value _exp
 		Token::Value currentTokenValue = m_scanner->currentToken();
 		if (currentTokenValue == Token::RBrace)
 			break;
-		else if (currentTokenValue == Token::Function)
+		else if (
+			currentTokenValue == Token::Function ||
+			(currentTokenValue == Token::Identifier && m_scanner->currentLiteral() == "constructor")
+		)
 			// This can be a function or a state variable of function type (especially
 			// complicated to distinguish fallback function from function type state variable)
 			subNodes.push_back(parseFunctionDefinitionOrFunctionTypeStateVariable(name.get()));
@@ -283,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)
@@ -329,15 +332,31 @@ StateMutability Parser::parseStateMutability(Token::Value _token)
 	return stateMutability;
 }
 
-Parser::FunctionHeaderParserResult Parser::parseFunctionHeader(bool _forceEmptyName, bool _allowModifiers)
+Parser::FunctionHeaderParserResult Parser::parseFunctionHeader(
+	bool _forceEmptyName,
+	bool _allowModifiers,
+	ASTString const* _contractName
+)
 {
 	RecursionGuard recursionGuard(*this);
 	FunctionHeaderParserResult result;
-	expectToken(Token::Function);
-	if (_forceEmptyName || m_scanner->currentToken() == Token::LParen)
-		result.name = make_shared<ASTString>(); // anonymous function
+
+	result.isConstructor = false;
+
+	if (m_scanner->currentToken() == Token::Identifier && m_scanner->currentLiteral() == "constructor")
+		result.isConstructor = true;
+	else if (m_scanner->currentToken() != Token::Function)
+		solAssert(false, "Function or constructor expected.");
+	m_scanner->next();
+
+	if (result.isConstructor || _forceEmptyName || m_scanner->currentToken() == Token::LParen)
+		result.name = make_shared<ASTString>();
 	else
 		result.name = expectIdentifierToken();
+
+	if (!result.name->empty() && _contractName && *result.name == *_contractName)
+		result.isConstructor = true;
+
 	VarDeclParserOptions options;
 	options.allowLocationSpecifier = true;
 	result.parameters = parseParameterList(options);
@@ -346,12 +365,13 @@ Parser::FunctionHeaderParserResult Parser::parseFunctionHeader(bool _forceEmptyN
 		Token::Value token = m_scanner->currentToken();
 		if (_allowModifiers && token == Token::Identifier)
 		{
-			// This can either be a modifier (function declaration) or the name of the
-			// variable (function type name plus variable).
-			if (
+			// If the name is empty (and this is not a constructor),
+			// then this can either be a modifier (fallback function declaration)
+			// or the name of the state variable (function type name plus variable).
+			if ((result.name->empty() && !result.isConstructor) && (
 				m_scanner->peekNextToken() == Token::Semicolon ||
 				m_scanner->peekNextToken() == Token::Assign
-			)
+			))
 				// Variable declaration, break here.
 				break;
 			else
@@ -361,6 +381,14 @@ Parser::FunctionHeaderParserResult Parser::parseFunctionHeader(bool _forceEmptyN
 		{
 			if (result.visibility != Declaration::Visibility::Default)
 			{
+				// There is the special case of a public state variable of function type.
+				// Detect this and return early.
+				if (
+					(result.visibility == Declaration::Visibility::External || result.visibility == Declaration::Visibility::Internal) &&
+					result.modifiers.empty() &&
+					(result.name->empty() && !result.isConstructor)
+				)
+					break;
 				parserError(string(
 					"Visibility already specified as \"" +
 					Declaration::visibilityToString(result.visibility) +
@@ -407,9 +435,10 @@ ASTPointer<ASTNode> Parser::parseFunctionDefinitionOrFunctionTypeStateVariable(A
 	if (m_scanner->currentCommentLiteral() != "")
 		docstring = make_shared<ASTString>(m_scanner->currentCommentLiteral());
 
-	FunctionHeaderParserResult header = parseFunctionHeader(false, true);
+	FunctionHeaderParserResult header = parseFunctionHeader(false, true, _contractName);
 
 	if (
+		header.isConstructor ||
 		!header.modifiers.empty() ||
 		!header.name->empty() ||
 		m_scanner->currentToken() == Token::Semicolon ||
@@ -426,12 +455,11 @@ ASTPointer<ASTNode> Parser::parseFunctionDefinitionOrFunctionTypeStateVariable(A
 		}
 		else
 			m_scanner->next(); // just consume the ';'
-		bool const c_isConstructor = (_contractName && *header.name == *_contractName);
 		return nodeFactory.createNode<FunctionDefinition>(
 			header.name,
 			header.visibility,
 			header.stateMutability,
-			c_isConstructor,
+			header.isConstructor,
 			docstring,
 			header.parameters,
 			header.modifiers,
@@ -579,8 +607,10 @@ ASTPointer<VariableDeclaration> Parser::parseVariableDeclaration(
 	if (_options.allowEmptyName && m_scanner->currentToken() != Token::Identifier)
 	{
 		identifier = make_shared<ASTString>("");
-		solAssert(type != nullptr, "");
-		nodeFactory.setEndPositionFromNode(type);
+		solAssert(!_options.allowVar, ""); // allowEmptyName && allowVar makes no sense
+		if (type)
+			nodeFactory.setEndPositionFromNode(type);
+		// if type is null this has already caused an error
 	}
 	else
 		identifier = expectIdentifierToken();
@@ -683,17 +713,17 @@ ASTPointer<ModifierInvocation> Parser::parseModifierInvocation()
 	RecursionGuard recursionGuard(*this);
 	ASTNodeFactory nodeFactory(*this);
 	ASTPointer<Identifier> name(parseIdentifier());
-	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<ModifierInvocation>(name, arguments);
+	return nodeFactory.createNode<ModifierInvocation>(name, move(arguments));
 }
 
 ASTPointer<Identifier> Parser::parseIdentifier()
@@ -776,6 +806,7 @@ ASTPointer<FunctionTypeName> Parser::parseFunctionType()
 	RecursionGuard recursionGuard(*this);
 	ASTNodeFactory nodeFactory(*this);
 	FunctionHeaderParserResult header = parseFunctionHeader(true, false);
+	solAssert(!header.isConstructor, "Tried to parse type as constructor.");
 	return nodeFactory.createNode<FunctionTypeName>(
 		header.parameters,
 		header.returnParameters,
diff --git a/libsolidity/parsing/Parser.h b/libsolidity/parsing/Parser.h
index 3f780af9..eb120a61 100644
--- a/libsolidity/parsing/Parser.h
+++ b/libsolidity/parsing/Parser.h
@@ -56,6 +56,7 @@ private:
 	/// This struct is shared for parsing a function header and a function type.
 	struct FunctionHeaderParserResult
 	{
+		bool isConstructor;
 		ASTPointer<ASTString> name;
 		ASTPointer<ParameterList> parameters;
 		ASTPointer<ParameterList> returnParameters;
@@ -73,7 +74,11 @@ private:
 	ASTPointer<InheritanceSpecifier> parseInheritanceSpecifier();
 	Declaration::Visibility parseVisibilitySpecifier(Token::Value _token);
 	StateMutability parseStateMutability(Token::Value _token);
-	FunctionHeaderParserResult parseFunctionHeader(bool _forceEmptyName, bool _allowModifiers);
+	FunctionHeaderParserResult parseFunctionHeader(
+		bool _forceEmptyName,
+		bool _allowModifiers,
+		ASTString const* _contractName = nullptr
+	);
 	ASTPointer<ASTNode> parseFunctionDefinitionOrFunctionTypeStateVariable(ASTString const* _contractName);
 	ASTPointer<FunctionDefinition> parseFunctionDefinition(ASTString const* _contractName);
 	ASTPointer<StructDefinition> parseStructDefinition();
diff --git a/libsolidity/parsing/Token.cpp b/libsolidity/parsing/Token.cpp
index 9cec0303..5ce74316 100644
--- a/libsolidity/parsing/Token.cpp
+++ b/libsolidity/parsing/Token.cpp
@@ -53,7 +53,7 @@ namespace solidity
 
 void ElementaryTypeNameToken::assertDetails(Token::Value _baseType, unsigned const& _first, unsigned const& _second)
 {
-	solAssert(Token::isElementaryTypeName(_baseType), "");
+	solAssert(Token::isElementaryTypeName(_baseType), "Expected elementary type name: " + string(Token::toString(_baseType)));
 	if (_baseType == Token::BytesM)
 	{
 		solAssert(_second == 0, "There should not be a second size argument to type bytesM.");