Disallow multi variable declarations with mismatching number of values.

6 files changed, 25 insertions, 98 deletions

diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index 8ea0a4d7..e2e4d9c3 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -1043,10 +1043,10 @@ namespace
  * @returns a suggested left-hand-side of a multi-variable declaration contairing
  * the variable declarations given in @a _decls.
  */
-string createTupleDecl(vector<VariableDeclaration const*> const& _decls)
+string createTupleDecl(vector<ASTPointer<VariableDeclaration>> const& _decls)
 {
 	vector<string> components;
-	for (VariableDeclaration const* decl: _decls)
+	for (ASTPointer<VariableDeclaration> const& decl: _decls)
 		if (decl)
 			components.emplace_back(decl->annotation().type->toString(false) + " " + decl->name());
 		else
@@ -1058,9 +1058,9 @@ string createTupleDecl(vector<VariableDeclaration const*> const& _decls)
 		return "(" + boost::algorithm::join(components, ", ") + ")";
 }
 
-bool typeCanBeExpressed(vector<VariableDeclaration const*> const& decls)
+bool typeCanBeExpressed(vector<ASTPointer<VariableDeclaration>> const& decls)
 {
-	for (VariableDeclaration const* decl: decls)
+	for (ASTPointer<VariableDeclaration> const& decl: decls)
 	{
 		// skip empty tuples (they can be expressed of course)
 		if (!decl)
@@ -1080,7 +1080,6 @@ bool typeCanBeExpressed(vector<VariableDeclaration const*> const& decls)
 
 bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
 {
-	bool const v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050);
 	if (!_statement.initialValue())
 	{
 		// No initial value is only permitted for single variables with specified type.
@@ -1119,82 +1118,27 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
 	else
 		valueTypes = TypePointers{type(*_statement.initialValue())};
 
-	// Determine which component is assigned to which variable.
-	// If numbers do not match, fill up if variables begin or end empty (not both).
-	vector<VariableDeclaration const*>& assignments = _statement.annotation().assignments;
-	assignments.resize(valueTypes.size(), nullptr);
 	vector<ASTPointer<VariableDeclaration>> const& variables = _statement.declarations();
 	if (variables.empty())
-	{
-		if (!valueTypes.empty())
-			m_errorReporter.fatalTypeError(
-				_statement.location(),
-				"Too many components (" +
-				toString(valueTypes.size()) +
-				") in value for variable assignment (0) needed"
-			);
-	}
+		// We already have an error for this in the SyntaxChecker.
+		solAssert(m_errorReporter.hasErrors(), "");
 	else if (valueTypes.size() != variables.size())
-	{
-		if (v050)
-			m_errorReporter.fatalTypeError(
-				_statement.location(),
-				"Different number of components on the left hand side (" +
-				toString(variables.size()) +
-				") than on the right hand side (" +
-				toString(valueTypes.size()) +
-				")."
-			);
-		else if (!variables.front() && !variables.back())
-			m_errorReporter.fatalTypeError(
-				_statement.location(),
-				"Wildcard both at beginning and end of variable declaration list is only allowed "
-				"if the number of components is equal."
-			);
-		else
-			m_errorReporter.warning(
-				_statement.location(),
-				"Different number of components on the left hand side (" +
-				toString(variables.size()) +
-				") than on the right hand side (" +
-				toString(valueTypes.size()) +
-				")."
-			);
-	}
-	size_t minNumValues = variables.size();
-	if (!variables.empty() && (!variables.back() || !variables.front()))
-		--minNumValues;
-	if (valueTypes.size() < minNumValues)
-		m_errorReporter.fatalTypeError(
-			_statement.location(),
-			"Not enough components (" +
-			toString(valueTypes.size()) +
-			") in value to assign all variables (" +
-			toString(minNumValues) + ")."
-		);
-	if (valueTypes.size() > variables.size() && variables.front() && variables.back())
-		m_errorReporter.fatalTypeError(
+		m_errorReporter.typeError(
 			_statement.location(),
-			"Too many components (" +
+			"Different number of components on the left hand side (" +
+			toString(variables.size()) +
+			") than on the right hand side (" +
 			toString(valueTypes.size()) +
-			") in value for variable assignment (" +
-			toString(minNumValues) +
-			" needed)."
+			")."
 		);
-	bool fillRight = !variables.empty() && (!variables.back() || variables.front());
-	for (size_t i = 0; i < min(variables.size(), valueTypes.size()); ++i)
-		if (fillRight)
-			assignments[i] = variables[i].get();
-		else
-			assignments[assignments.size() - i - 1] = variables[variables.size() - i - 1].get();
 
 	bool autoTypeDeductionNeeded = false;
 
-	for (size_t i = 0; i < assignments.size(); ++i)
+	for (size_t i = 0; i < min(variables.size(), valueTypes.size()); ++i)
 	{
-		if (!assignments[i])
+		if (!variables[i])
 			continue;
-		VariableDeclaration const& var = *assignments[i];
+		VariableDeclaration const& var = *variables[i];
 		solAssert(!var.value(), "Value has to be tied to statement.");
 		TypePointer const& valueComponentType = valueTypes[i];
 		solAssert(!!valueComponentType, "");
@@ -1284,7 +1228,7 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
 
 	if (autoTypeDeductionNeeded)
 	{
-		if (!typeCanBeExpressed(assignments))
+		if (!typeCanBeExpressed(variables))
 			m_errorReporter.syntaxError(
 				_statement.location(),
 				"Use of the \"var\" keyword is disallowed. "
@@ -1294,7 +1238,7 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
 			m_errorReporter.syntaxError(
 				_statement.location(),
 				"Use of the \"var\" keyword is disallowed. "
-				"Use explicit declaration `" + createTupleDecl(assignments) + " = ...´ instead."
+				"Use explicit declaration `" + createTupleDecl(variables) + " = ...´ instead."
 			);
 	}
 
diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp
index 16c9b2d2..7719d080 100644
--- a/libsolidity/ast/AST.cpp
+++ b/libsolidity/ast/AST.cpp
@@ -535,13 +535,6 @@ ReturnAnnotation& Return::annotation() const
 	return dynamic_cast<ReturnAnnotation&>(*m_annotation);
 }
 
-VariableDeclarationStatementAnnotation& VariableDeclarationStatement::annotation() const
-{
-	if (!m_annotation)
-		m_annotation = new VariableDeclarationStatementAnnotation();
-	return dynamic_cast<VariableDeclarationStatementAnnotation&>(*m_annotation);
-}
-
 ExpressionAnnotation& Expression::annotation() const
 {
 	if (!m_annotation)
diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h
index e862fd62..acd90ad8 100644
--- a/libsolidity/ast/AST.h
+++ b/libsolidity/ast/AST.h
@@ -1270,8 +1270,6 @@ public:
 	virtual void accept(ASTVisitor& _visitor) override;
 	virtual void accept(ASTConstVisitor& _visitor) const override;
 
-	VariableDeclarationStatementAnnotation& annotation() const override;
-
 	std::vector<ASTPointer<VariableDeclaration>> const& declarations() const { return m_variables; }
 	Expression const* initialValue() const { return m_initialValue.get(); }
 
diff --git a/libsolidity/ast/ASTAnnotations.h b/libsolidity/ast/ASTAnnotations.h
index 5cbe42bd..e0b3f492 100644
--- a/libsolidity/ast/ASTAnnotations.h
+++ b/libsolidity/ast/ASTAnnotations.h
@@ -164,13 +164,6 @@ struct UserDefinedTypeNameAnnotation: TypeNameAnnotation
 	ContractDefinition const* contractScope = nullptr;
 };
 
-struct VariableDeclarationStatementAnnotation: StatementAnnotation
-{
-	/// Information about which component of the value is assigned to which variable.
-	/// The pointer can be null to signify that the component is discarded.
-	std::vector<VariableDeclaration const*> assignments;
-};
-
 struct ExpressionAnnotation: ASTAnnotation
 {
 	/// Inferred type of the expression.
diff --git a/libsolidity/ast/ASTJsonConverter.cpp b/libsolidity/ast/ASTJsonConverter.cpp
index b7855668..a26828a6 100644
--- a/libsolidity/ast/ASTJsonConverter.cpp
+++ b/libsolidity/ast/ASTJsonConverter.cpp
@@ -555,8 +555,8 @@ bool ASTJsonConverter::visit(EmitStatement const& _node)
 bool ASTJsonConverter::visit(VariableDeclarationStatement const& _node)
 {
 	Json::Value varDecs(Json::arrayValue);
-	for (auto const& v: _node.annotation().assignments)
-		appendMove(varDecs, idOrNull(v));
+	for (auto const& v: _node.declarations())
+		appendMove(varDecs, idOrNull(v.get()));
 	setJsonNode(_node, "VariableDeclarationStatement", {
 		make_pair("assignments", std::move(varDecs)),
 		make_pair("declarations", toJson(_node.declarations())),
diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp
index 93f698bc..bbb3db3d 100644
--- a/libsolidity/codegen/ContractCompiler.cpp
+++ b/libsolidity/codegen/ContractCompiler.cpp
@@ -833,20 +833,19 @@ bool ContractCompiler::visit(VariableDeclarationStatement const& _variableDeclar
 			valueTypes = tupleType->components();
 		else
 			valueTypes = TypePointers{expression->annotation().type};
-		auto const& assignments = _variableDeclarationStatement.annotation().assignments;
-		solAssert(assignments.size() == valueTypes.size(), "");
-		for (size_t i = 0; i < assignments.size(); ++i)
+		auto const& declarations = _variableDeclarationStatement.declarations();
+		solAssert(declarations.size() == valueTypes.size(), "");
+		for (size_t i = 0; i < declarations.size(); ++i)
 		{
-			size_t j = assignments.size() - i - 1;
+			size_t j = declarations.size() - i - 1;
 			solAssert(!!valueTypes[j], "");
-			VariableDeclaration const* varDecl = assignments[j];
-			if (!varDecl)
-				utils.popStackElement(*valueTypes[j]);
-			else
+			if (VariableDeclaration const* varDecl = declarations[j].get())
 			{
 				utils.convertType(*valueTypes[j], *varDecl->annotation().type);
 				utils.moveToStackVariable(*varDecl);
 			}
+			else
+				utils.popStackElement(*valueTypes[j]);
 		}
 	}
 	checker.check();