Merge pull request #9428 from ethereum/removeVar

Remove special treatment of ``var``.

libsolidity/analysis/DeclarationTypeChecker.cpp

@@ -293,15 +293,6 @@ void DeclarationTypeChecker::endVisit(VariableDeclaration const& _variable)
 			"The \"immutable\" keyword can only be used for state variables."
 		);
 
-	if (!_variable.typeName())
-	{
-		// This can still happen in very unusual cases where a developer uses constructs, such as
-		// `var a;`, however, such code will have generated errors already.
-		// However, we cannot blindingly solAssert() for that here, as the TypeChecker (which is
-		// invoking ReferencesResolver) is generating it, so the error is most likely(!) generated
-		// after this step.
-		return;
-	}
 	using Location = VariableDeclaration::Location;
 	Location varLoc = _variable.referenceLocation();
 	DataLocation typeLoc = DataLocation::Memory;
@@ -382,7 +373,7 @@ void DeclarationTypeChecker::endVisit(VariableDeclaration const& _variable)
 				solAssert(!_variable.hasReferenceOrMappingType(), "Data location not properly set.");
 		}
 
-	TypePointer type = _variable.typeName()->annotation().type;
+	TypePointer type = _variable.typeName().annotation().type;
 	if (auto ref = dynamic_cast<ReferenceType const*>(type))
 	{
 		bool isPointer = !_variable.isStateVariable();

libsolidity/analysis/TypeChecker.cpp

@@ -466,8 +466,7 @@ bool TypeChecker::visit(FunctionDefinition const& _function)
 
 bool TypeChecker::visit(VariableDeclaration const& _variable)
 {
-	if (_variable.typeName())
-		_variable.typeName()->accept(*this);
+	_variable.typeName().accept(*this);
 
 	// type is filled either by ReferencesResolver directly from the type name or by
 	// TypeChecker at the VariableDeclarationStatement level.
@@ -591,15 +590,11 @@ bool TypeChecker::visit(VariableDeclaration const& _variable)
 			if (_variable.isStateVariable())
 				m_errorReporter.warning(3408_error, _variable.location(), collisionMessage(_variable.name(), true));
 			else
-				m_errorReporter.warning(
-					2332_error,
-					_variable.typeName() ? _variable.typeName()->location() : _variable.location(),
-					collisionMessage(varType->canonicalName(), false)
-				);
+				m_errorReporter.warning(2332_error, _variable.typeName().location(), collisionMessage(varType->canonicalName(), false));
 		}
 		vector<Type const*> oversizedSubtypes = frontend::oversizedSubtypes(*varType);
 		for (Type const* subtype: oversizedSubtypes)
-			m_errorReporter.warning(7325_error, _variable.typeName()->location(), collisionMessage(subtype->canonicalName(), false));
+			m_errorReporter.warning(7325_error, _variable.typeName().location(), collisionMessage(subtype->canonicalName(), false));
 	}
 
 	return false;
@@ -1106,81 +1101,23 @@ void TypeChecker::endVisit(EmitStatement const& _emit)
 		m_errorReporter.typeError(9292_error, _emit.eventCall().expression().location(), "Expression has to be an event invocation.");
 }
 
-namespace
-{
-/**
- * @returns a suggested left-hand-side of a multi-variable declaration contairing
- * the variable declarations given in @a _decls.
- */
-string createTupleDecl(vector<ASTPointer<VariableDeclaration>> const& _decls)
-{
-	vector<string> components;
-	for (ASTPointer<VariableDeclaration> const& decl: _decls)
-		if (decl)
-		{
-			solAssert(decl->annotation().type, "");
-			components.emplace_back(decl->annotation().type->toString(false) + " " + decl->name());
-		}
-		else
-			components.emplace_back();
-
-	if (_decls.size() == 1)
-		return components.front();
-	else
-		return "(" + boost::algorithm::join(components, ", ") + ")";
-}
-
-bool typeCanBeExpressed(vector<ASTPointer<VariableDeclaration>> const& decls)
-{
-	for (ASTPointer<VariableDeclaration> const& decl: decls)
-	{
-		// skip empty tuples (they can be expressed of course)
-		if (!decl)
-			continue;
-
-		if (!decl->annotation().type)
-			return false;
-
-		if (auto functionType = dynamic_cast<FunctionType const*>(decl->annotation().type))
-			if (
-				functionType->kind() != FunctionType::Kind::Internal &&
-				functionType->kind() != FunctionType::Kind::External
-			)
-				return false;
-	}
-
-	return true;
-}
-}
-
 bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
 {
 	if (!_statement.initialValue())
 	{
 		// No initial value is only permitted for single variables with specified type.
+		// This usually already results in a parser error.
 		if (_statement.declarations().size() != 1 || !_statement.declarations().front())
 		{
-			if (std::all_of(
-				_statement.declarations().begin(),
-				_statement.declarations().end(),
-				[](ASTPointer<VariableDeclaration> const& declaration) { return declaration == nullptr; }
-			))
-			{
-				// The syntax checker has already generated an error for this case (empty LHS tuple).
-				solAssert(m_errorReporter.hasErrors(), "");
+			solAssert(m_errorReporter.hasErrors(), "");
 
-				// It is okay to return here, as there are no named components on the
-				// left-hand-side that could cause any damage later.
-				return false;
-			}
-			else
-				// Bailing out *fatal* here, as those (untyped) vars may be used later, and diagnostics wouldn't be helpful then.
-				m_errorReporter.fatalTypeError(4626_error, _statement.location(), "Use of the \"var\" keyword is disallowed.");
+			// It is okay to return here, as there are no named components on the
+			// left-hand-side that could cause any damage later.
+			return false;
 		}
 
 		VariableDeclaration const& varDecl = *_statement.declarations().front();
-		if (!varDecl.annotation().type)
-			m_errorReporter.fatalTypeError(6983_error, _statement.location(), "Use of the \"var\" keyword is disallowed.");
+		solAssert(varDecl.annotation().type, "");
 
 		if (dynamic_cast<MappingType const*>(type(varDecl)))
 			m_errorReporter.typeError(
@@ -1217,8 +1154,6 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
 			")."
 		);
 
-	bool autoTypeDeductionNeeded = false;
-
 	for (size_t i = 0; i < min(variables.size(), valueTypes.size()); ++i)
 	{
 		if (!variables[i])
@@ -1227,95 +1162,45 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
 		solAssert(!var.value(), "Value has to be tied to statement.");
 		TypePointer const& valueComponentType = valueTypes[i];
 		solAssert(!!valueComponentType, "");
-		if (!var.annotation().type)
-		{
-			autoTypeDeductionNeeded = true;
+		solAssert(var.annotation().type, "");
 
-			// Infer type from value.
-			solAssert(!var.typeName(), "");
-			var.annotation().type = valueComponentType->mobileType();
-			if (!var.annotation().type)
-			{
-				if (valueComponentType->category() == Type::Category::RationalNumber)
-					m_errorReporter.fatalTypeError(
-						6963_error,
-						_statement.initialValue()->location(),
-						"Invalid rational " +
-						valueComponentType->toString() +
-						" (absolute value too large or division by zero)."
-					);
-				else
-					solAssert(false, "");
-			}
-			else if (*var.annotation().type == *TypeProvider::emptyTuple())
-				solAssert(false, "Cannot declare variable with void (empty tuple) type.");
-			else if (valueComponentType->category() == Type::Category::RationalNumber)
-			{
-				string typeName = var.annotation().type->toString(true);
-				string extension;
-				if (auto type = dynamic_cast<IntegerType const*>(var.annotation().type))
-				{
-					unsigned numBits = type->numBits();
-					bool isSigned = type->isSigned();
-					solAssert(numBits > 0, "");
-					string minValue;
-					string maxValue;
-					if (isSigned)
-					{
-						numBits--;
-						minValue = "-" + bigint(bigint(1) << numBits).str();
-					}
-					else
-						minValue = "0";
-					maxValue = bigint((bigint(1) << numBits) - 1).str();
-					extension = ", which can hold values between " + minValue + " and " + maxValue;
-				}
-				else
-					solAssert(dynamic_cast<FixedPointType const*>(var.annotation().type), "Unknown type.");
-			}
-
-			var.accept(*this);
-		}
-		else
+		var.accept(*this);
+		BoolResult result = valueComponentType->isImplicitlyConvertibleTo(*var.annotation().type);
+		if (!result)
 		{
-			var.accept(*this);
-			BoolResult result = valueComponentType->isImplicitlyConvertibleTo(*var.annotation().type);
-			if (!result)
+			auto errorMsg = "Type " +
+				valueComponentType->toString() +
+				" is not implicitly convertible to expected type " +
+				var.annotation().type->toString();
+			if (
+				valueComponentType->category() == Type::Category::RationalNumber &&
+				dynamic_cast<RationalNumberType const&>(*valueComponentType).isFractional() &&
+				valueComponentType->mobileType()
+			)
 			{
-				auto errorMsg = "Type " +
-					valueComponentType->toString() +
-					" is not implicitly convertible to expected type " +
-					var.annotation().type->toString();
-				if (
-					valueComponentType->category() == Type::Category::RationalNumber &&
-					dynamic_cast<RationalNumberType const&>(*valueComponentType).isFractional() &&
-					valueComponentType->mobileType()
-				)
-				{
-					if (var.annotation().type->operator==(*valueComponentType->mobileType()))
-						m_errorReporter.typeError(
-							5107_error,
-							_statement.location(),
-							errorMsg + ", but it can be explicitly converted."
-						);
-					else
-						m_errorReporter.typeError(
-							4486_error,
-							_statement.location(),
-							errorMsg +
-							". Try converting to type " +
-							valueComponentType->mobileType()->toString() +
-							" or use an explicit conversion."
-						);
-				}
-				else
-					m_errorReporter.typeErrorConcatenateDescriptions(
-						9574_error,
+				if (var.annotation().type->operator==(*valueComponentType->mobileType()))
+					m_errorReporter.typeError(
+						5107_error,
 						_statement.location(),
-						errorMsg + ".",
-						result.message()
+						errorMsg + ", but it can be explicitly converted."
+					);
+				else
+					m_errorReporter.typeError(
+						4486_error,
+						_statement.location(),
+						errorMsg +
+						". Try converting to type " +
+						valueComponentType->mobileType()->toString() +
+						" or use an explicit conversion."
 					);
 			}
+			else
+				m_errorReporter.typeErrorConcatenateDescriptions(
+					9574_error,
+					_statement.location(),
+					errorMsg + ".",
+					result.message()
+				);
 		}
 	}
 
@@ -1327,24 +1212,6 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
 				BOOST_THROW_EXCEPTION(FatalError());
 	}
 
-	if (autoTypeDeductionNeeded)
-	{
-		if (!typeCanBeExpressed(variables))
-			m_errorReporter.syntaxError(
-				3478_error,
-				_statement.location(),
-				"Use of the \"var\" keyword is disallowed. "
-				"Type cannot be expressed in syntax."
-			);
-		else
-			m_errorReporter.syntaxError(
-				1719_error,
-				_statement.location(),
-				"Use of the \"var\" keyword is disallowed. "
-				"Use explicit declaration `" + createTupleDecl(variables) + " = ...´ instead."
-			);
-	}
-
 	return false;
 }
 

libsolidity/ast/AST.cpp

@@ -614,9 +614,8 @@ bool VariableDeclaration::isEventParameter() const
 
 bool VariableDeclaration::hasReferenceOrMappingType() const
 {
-	solAssert(typeName(), "");
-	solAssert(typeName()->annotation().type, "Can only be called after reference resolution");
-	Type const* type = typeName()->annotation().type;
+	solAssert(typeName().annotation().type, "Can only be called after reference resolution");
+	Type const* type = typeName().annotation().type;
 	return type->category() == Type::Category::Mapping || dynamic_cast<ReferenceType const*>(type);
 }
 
@@ -642,22 +641,8 @@ set<VariableDeclaration::Location> VariableDeclaration::allowedDataLocations() c
 		return locations;
 	}
 	else if (isLocalVariable())
-	{
-		solAssert(typeName(), "");
-		auto dataLocations = [](TypePointer _type, auto&& _recursion) -> set<Location> {
-			solAssert(_type, "Can only be called after reference resolution");
-			switch (_type->category())
-			{
-				case Type::Category::Array:
-					return _recursion(dynamic_cast<ArrayType const*>(_type)->baseType(), _recursion);
-				case Type::Category::Mapping:
-					return set<Location>{ Location::Storage };
-				default:
-					return set<Location>{ Location::Memory, Location::Storage, Location::CallData };
-			}
-		};
-		return dataLocations(typeName()->annotation().type, dataLocations);
-	}
+		// Further restrictions will be imposed later on.
+		return set<Location>{ Location::Memory, Location::Storage, Location::CallData };
 	else
 		// Struct members etc.
 		return set<Location>{ Location::Unspecified };

libsolidity/ast/AST.h

@@ -895,13 +895,16 @@ public:
 		m_isIndexed(_isIndexed),
 		m_mutability(_mutability),
 		m_overrides(std::move(_overrides)),
-		m_location(_referenceLocation) {}
+		m_location(_referenceLocation)
+	{
+		solAssert(m_typeName, "");
+	}
 
 
 	void accept(ASTVisitor& _visitor) override;
 	void accept(ASTConstVisitor& _visitor) const override;
 
-	TypeName* typeName() const { return m_typeName.get(); }
+	TypeName const& typeName() const { return *m_typeName; }
 	ASTPointer<Expression> const& value() const { return m_value; }
 
 	bool isLValue() const override;
@@ -964,7 +967,7 @@ protected:
 	Visibility defaultVisibility() const override { return Visibility::Internal; }
 
 private:
-	ASTPointer<TypeName> m_typeName; ///< can be empty ("var")
+	ASTPointer<TypeName> m_typeName;
 	/// Initially assigned value, can be missing. For local variables, this is stored inside
 	/// VariableDeclarationStatement and not here.
 	ASTPointer<Expression> m_value;

libsolidity/ast/ASTJsonConverter.cpp

@@ -386,7 +386,7 @@ bool ASTJsonConverter::visit(VariableDeclaration const& _node)
 {
 	std::vector<pair<string, Json::Value>> attributes = {
 		make_pair("name", _node.name()),
-		make_pair("typeName", toJsonOrNull(_node.typeName())),
+		make_pair("typeName", toJson(_node.typeName())),
 		make_pair("constant", _node.isConstant()),
 		make_pair("mutability", VariableDeclaration::mutabilityToString(_node.mutability())),
 		make_pair("stateVariable", _node.isStateVariable()),

libsolidity/parsing/Parser.cpp

@@ -677,16 +677,10 @@ ASTPointer<VariableDeclaration> Parser::parseVariableDeclaration(
 	RecursionGuard recursionGuard(*this);
 	ASTNodeFactory nodeFactory = _lookAheadArrayType ?
 		ASTNodeFactory(*this, _lookAheadArrayType) : ASTNodeFactory(*this);
-	ASTPointer<TypeName> type;
+
 	ASTPointer<StructuredDocumentation> const documentation = parseStructuredDocumentation();
-	if (_lookAheadArrayType)
-		type = _lookAheadArrayType;
-	else
-	{
-		type = parseTypeName(_options.allowVar);
-		if (type != nullptr)
-			nodeFactory.setEndPositionFromNode(type);
-	}
+	ASTPointer<TypeName> type = _lookAheadArrayType ? _lookAheadArrayType : parseTypeName();
+	nodeFactory.setEndPositionFromNode(type);
 
 	if (!_options.isStateVariable && documentation != nullptr)
 		parserError(2837_error, "Only state variables can have a docstring.");
@@ -753,8 +747,6 @@ ASTPointer<VariableDeclaration> Parser::parseVariableDeclaration(
 			{
 				if (location != VariableDeclaration::Location::Unspecified)
 					parserError(3548_error, "Location already specified.");
-				else if (!type)
-					parserError(7439_error, "Location specifier needs explicit type name.");
 				else
 				{
 					switch (token)
@@ -781,10 +773,7 @@ ASTPointer<VariableDeclaration> Parser::parseVariableDeclaration(
 	}
 
 	if (_options.allowEmptyName && m_scanner->currentToken() != Token::Identifier)
-	{
 		identifier = make_shared<ASTString>("");
-		solAssert(!_options.allowVar, ""); // allowEmptyName && allowVar makes no sense
-	}
 	else
 	{
 		nodeFactory.markEndPosition();
@@ -908,7 +897,7 @@ ASTPointer<UsingForDirective> Parser::parseUsingDirective()
 	if (m_scanner->currentToken() == Token::Mul)
 		m_scanner->next();
 	else
-		typeName = parseTypeName(false);
+		typeName = parseTypeName();
 	nodeFactory.markEndPosition();
 	expectToken(Token::Semicolon);
 	return nodeFactory.createNode<UsingForDirective>(library, typeName);
@@ -971,7 +960,7 @@ ASTPointer<TypeName> Parser::parseTypeNameSuffix(ASTPointer<TypeName> type, ASTN
 	return type;
 }
 
-ASTPointer<TypeName> Parser::parseTypeName(bool _allowVar)
+ASTPointer<TypeName> Parser::parseTypeName()
 {
 	RecursionGuard recursionGuard(*this);
 	ASTNodeFactory nodeFactory(*this);
@@ -1004,12 +993,6 @@ ASTPointer<TypeName> Parser::parseTypeName(bool _allowVar)
 		}
 		type = nodeFactory.createNode<ElementaryTypeName>(elemTypeName, stateMutability);
 	}
-	else if (token == Token::Var)
-	{
-		if (!_allowVar)
-			parserError(7059_error, "Expected explicit type name.");
-		m_scanner->next();
-	}
 	else if (token == Token::Function)
 		type = parseFunctionType();
 	else if (token == Token::Mapping)
@@ -1019,9 +1002,10 @@ ASTPointer<TypeName> Parser::parseTypeName(bool _allowVar)
 	else
 		fatalParserError(3546_error, "Expected type name");
 
-	if (type)
-		// Parse "[...]" postfixes for arrays.
-		type = parseTypeNameSuffix(type, nodeFactory);
+	solAssert(type, "");
+	// Parse "[...]" postfixes for arrays.
+	type = parseTypeNameSuffix(type, nodeFactory);
+
 	return type;
 }
 
@@ -1062,8 +1046,7 @@ ASTPointer<Mapping> Parser::parseMapping()
 	else
 		fatalParserError(1005_error, "Expected elementary type name or identifier for mapping key type");
 	expectToken(Token::Arrow);
-	bool const allowVar = false;
-	ASTPointer<TypeName> valueType = parseTypeName(allowVar);
+	ASTPointer<TypeName> valueType = parseTypeName();
 	nodeFactory.markEndPosition();
 	expectToken(Token::RParen);
 	return nodeFactory.createNode<Mapping>(keyType, valueType);
@@ -1544,53 +1527,14 @@ ASTPointer<VariableDeclarationStatement> Parser::parseVariableDeclarationStateme
 	ASTNodeFactory nodeFactory(*this);
 	if (_lookAheadArrayType)
 		nodeFactory.setLocation(_lookAheadArrayType->location());
+
+	VarDeclParserOptions options;
+	options.allowLocationSpecifier = true;
 	vector<ASTPointer<VariableDeclaration>> variables;
+	variables.emplace_back(parseVariableDeclaration(options, _lookAheadArrayType));
+	nodeFactory.setEndPositionFromNode(variables.back());
+
 	ASTPointer<Expression> value;
-	if (
-		!_lookAheadArrayType &&
-		m_scanner->currentToken() == Token::Var &&
-		m_scanner->peekNextToken() == Token::LParen
-	)
-	{
-		// Parse `var (a, b, ,, c) = ...` into a single VariableDeclarationStatement with multiple variables.
-		m_scanner->next();
-		m_scanner->next();
-		if (m_scanner->currentToken() != Token::RParen)
-			while (true)
-			{
-				ASTPointer<VariableDeclaration> var;
-				if (
-					m_scanner->currentToken() != Token::Comma &&
-					m_scanner->currentToken() != Token::RParen
-				)
-				{
-					ASTNodeFactory varDeclNodeFactory(*this);
-					varDeclNodeFactory.markEndPosition();
-					ASTPointer<ASTString> name = expectIdentifierToken();
-					var = varDeclNodeFactory.createNode<VariableDeclaration>(
-						ASTPointer<TypeName>(),
-						name,
-						ASTPointer<Expression>(),
-						Visibility::Default
-					);
-				}
-				variables.push_back(var);
-				if (m_scanner->currentToken() == Token::RParen)
-					break;
-				else
-					expectToken(Token::Comma);
-			}
-		nodeFactory.markEndPosition();
-		m_scanner->next();
-	}
-	else
-	{
-		VarDeclParserOptions options;
-		options.allowVar = true;
-		options.allowLocationSpecifier = true;
-		variables.push_back(parseVariableDeclaration(options, _lookAheadArrayType));
-		nodeFactory.setEndPositionFromNode(variables.back());
-	}
 	if (m_scanner->currentToken() == Token::Assign)
 	{
 		m_scanner->next();
@@ -1704,11 +1648,8 @@ ASTPointer<Expression> Parser::parseLeftHandSideExpression(
 	else if (m_scanner->currentToken() == Token::New)
 	{
 		expectToken(Token::New);
-		ASTPointer<TypeName> typeName(parseTypeName(false));
-		if (typeName)
-			nodeFactory.setEndPositionFromNode(typeName);
-		else
-			nodeFactory.markEndPosition();
+		ASTPointer<TypeName> typeName(parseTypeName());
+		nodeFactory.setEndPositionFromNode(typeName);
 		expression = nodeFactory.createNode<NewExpression>(typeName);
 	}
 	else if (m_scanner->currentToken() == Token::Payable)
@@ -2048,7 +1989,7 @@ Parser::LookAheadInfo Parser::peekStatementType() const
 	Token token(m_scanner->currentToken());
 	bool mightBeTypeName = (TokenTraits::isElementaryTypeName(token) || token == Token::Identifier);
 
-	if (token == Token::Mapping || token == Token::Function || token == Token::Var)
+	if (token == Token::Mapping || token == Token::Function)
 		return LookAheadInfo::VariableDeclaration;
 	if (mightBeTypeName)
 	{

libsolidity/parsing/Parser.h

@@ -57,7 +57,6 @@ private:
 		// https://stackoverflow.com/questions/17430377
 		VarDeclParserOptions() {}
 
-		bool allowVar = false;
 		bool isStateVariable = false;
 		bool allowIndexed = false;
 		bool allowEmptyName = false;
@@ -107,7 +106,7 @@ private:
 	ASTPointer<Identifier> parseIdentifier();
 	ASTPointer<UserDefinedTypeName> parseUserDefinedTypeName();
 	ASTPointer<TypeName> parseTypeNameSuffix(ASTPointer<TypeName> type, ASTNodeFactory& nodeFactory);
-	ASTPointer<TypeName> parseTypeName(bool _allowVar);
+	ASTPointer<TypeName> parseTypeName();
 	ASTPointer<FunctionTypeName> parseFunctionType();
 	ASTPointer<Mapping> parseMapping();
 	ASTPointer<ParameterList> parseParameterList(