Merge pull request #4465 from ethereum/tupleAssignment

Disallow tuple assignment with mismatching number of components.

11 files changed, 23 insertions, 90 deletions

diff --git a/Changelog.md b/Changelog.md
index 304f70ea..2ded6bf2 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -29,6 +29,7 @@ Breaking Changes:
  * Optimizer: Remove the no-op ``PUSH1 0 NOT AND`` sequence.
  * Parser: Disallow trailing dots that are not followed by a number.
  * Parser: Remove ``constant`` as function state mutability modifer.
+ * Type Checker: Disallow assignments between tuples with different numbers of components. This was already the case in the experimental 0.5.0 mode.
  * Type Checker: Disallow values for constants that are not compile-time constants. This was already the case in the experimental 0.5.0 mode.
  * Type Checker: Disallow arithmetic operations for boolean variables.
  * Type Checker: Disallow conversions between ``bytesX`` and ``uintY`` of different size.
diff --git a/docs/control-structures.rst b/docs/control-structures.rst
index 8ced0fbc..7beca65e 100644
--- a/docs/control-structures.rst
+++ b/docs/control-structures.rst
@@ -293,11 +293,6 @@ These can then either be assigned to newly declared variables or to pre-existing
             (x, y) = (y, x);
             // Components can be left out (also for variable declarations).
             (data.length,,) = f(); // Sets the length to 7
-            // Components can only be left out at the left-hand-side of assignments, with
-            // one exception:
-            (x,) = (1,);
-            // (1,) is the only way to specify a 1-component tuple, because (1) is
-            // equivalent to 1.
         }
     }
 
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index ed7f05f7..20e423e6 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -1337,7 +1337,6 @@ bool TypeChecker::visit(Conditional const& _conditional)
 
 bool TypeChecker::visit(Assignment const& _assignment)
 {
-	bool const v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050);
 	requireLValue(_assignment.leftHandSide());
 	TypePointer t = type(_assignment.leftHandSide());
 	_assignment.annotation().type = t;
@@ -1354,25 +1353,8 @@ bool TypeChecker::visit(Assignment const& _assignment)
 		expectType(_assignment.rightHandSide(), *tupleType);
 
 		// expectType does not cause fatal errors, so we have to check again here.
-		if (TupleType const* rhsType = dynamic_cast<TupleType const*>(type(_assignment.rightHandSide()).get()))
-		{
+		if (dynamic_cast<TupleType const*>(type(_assignment.rightHandSide()).get()))
 			checkDoubleStorageAssignment(_assignment);
-			// @todo For 0.5.0, this code shoud move to TupleType::isImplicitlyConvertibleTo,
-			// but we cannot do it right now.
-			if (rhsType->components().size() != tupleType->components().size())
-			{
-				string message =
-					"Different number of components on the left hand side (" +
-					toString(tupleType->components().size()) +
-					") than on the right hand side (" +
-					toString(rhsType->components().size()) +
-					").";
-				if (v050)
-					m_errorReporter.typeError(_assignment.location(), message);
-				else
-					m_errorReporter.warning(_assignment.location(), message);
-			}
-		}
 	}
 	else if (t->category() == Type::Category::Mapping)
 	{
diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp
index 1c4eb76e..23614e58 100644
--- a/libsolidity/ast/Types.cpp
+++ b/libsolidity/ast/Types.cpp
@@ -2238,25 +2238,13 @@ bool TupleType::isImplicitlyConvertibleTo(Type const& _other) const
 		TypePointers const& targets = tupleType->components();
 		if (targets.empty())
 			return components().empty();
-		if (components().size() != targets.size() && !targets.front() && !targets.back())
-			return false; // (,a,) = (1,2,3,4) - unable to position `a` in the tuple.
-		size_t minNumValues = targets.size();
-		if (!targets.back() || !targets.front())
-			--minNumValues; // wildcards can also match 0 components
-		if (components().size() < minNumValues)
+		if (components().size() != targets.size())
 			return false;
-		if (components().size() > targets.size() && targets.front() && targets.back())
-			return false; // larger source and no wildcard
-		bool fillRight = !targets.back() || targets.front();
-		for (size_t i = 0; i < min(targets.size(), components().size()); ++i)
-		{
-			auto const& s = components()[fillRight ? i : components().size() - i - 1];
-			auto const& t = targets[fillRight ? i : targets.size() - i - 1];
-			if (!s && t)
+		for (size_t i = 0; i < targets.size(); ++i)
+			if (!components()[i] && targets[i])
 				return false;
-			else if (s && t && !s->isImplicitlyConvertibleTo(*t))
+			else if (components()[i] && targets[i] && !components()[i]->isImplicitlyConvertibleTo(*targets[i]))
 				return false;
-		}
 		return true;
 	}
 	else
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index bc613868..ce00931b 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -7895,7 +7895,7 @@ BOOST_AUTO_TEST_CASE(tuples)
 				if (a != 1 || b != 2 || c[0] != 3) return 2;
 				(a, b) = (b, a);
 				if (a != 2 || b != 1) return 3;
-				(a, , b, ) = (8, 9, 10, 11, 12);
+				(a, , b, , ) = (8, 9, 10, 11, 12);
 				if (a != 8 || b != 10) return 4;
 			}
 		}
@@ -7983,7 +7983,7 @@ BOOST_AUTO_TEST_CASE(destructuring_assignment)
 				if (loc != 3) return 9;
 				if (memArray.length != arrayData.length) return 10;
 				bytes memory memBytes;
-				(x, memBytes, y[2], ) = (456, s, 789, 101112, 131415);
+				(x, memBytes, y[2], , ) = (456, s, 789, 101112, 131415);
 				if (x != 456 || memBytes.length != s.length || y[2] != 789) return 11;
 			}
 		}
@@ -7992,31 +7992,6 @@ BOOST_AUTO_TEST_CASE(destructuring_assignment)
 	ABI_CHECK(callContractFunction("f(bytes)", u256(0x20), u256(5), string("abcde")), encodeArgs(u256(0)));
 }
 
-BOOST_AUTO_TEST_CASE(destructuring_assignment_wildcard)
-{
-	char const* sourceCode = R"(
-		contract C {
-			function f() public returns (uint) {
-				uint a;
-				uint b;
-				uint c;
-				(a,) = (1,);
-				if (a != 1) return 1;
-				(,b) = (2,3,4);
-				if (b != 4) return 2;
-				(, c,) = (5,6,7);
-				if (c != 6) return 3;
-				(a, b,) = (11, 12, 13);
-				if (a != 11 || b != 12) return 4;
-				(, a, b) = (11, 12, 13);
-				if (a != 12 || b != 13) return 5;
-			}
-		}
-	)";
-	compileAndRun(sourceCode);
-	ABI_CHECK(callContractFunction("f()"), encodeArgs(u256(0)));
-}
-
 BOOST_AUTO_TEST_CASE(lone_struct_array_type)
 {
 	char const* sourceCode = R"(
diff --git a/test/libsolidity/syntaxTests/tupleAssignments/err_fill_assignment.sol b/test/libsolidity/syntaxTests/tupleAssignments/err_fill_assignment.sol
new file mode 100644
index 00000000..32b381bb
--- /dev/null
+++ b/test/libsolidity/syntaxTests/tupleAssignments/err_fill_assignment.sol
@@ -0,0 +1,11 @@
+contract C {
+	function f() public pure returns (uint, uint, bytes32) {
+		uint a;
+		bytes32 b;
+		(a,) = f();
+		(,b) = f();
+	}
+}
+// ----
+// TypeError: (103-106): Type tuple(uint256,uint256,bytes32) is not implicitly convertible to expected type tuple(uint256,).
+// TypeError: (117-120): Type tuple(uint256,uint256,bytes32) is not implicitly convertible to expected type tuple(,bytes32).
diff --git a/test/libsolidity/syntaxTests/tupleAssignments/warn_multiple_storage_storage_copies_fill_left.sol b/test/libsolidity/syntaxTests/tupleAssignments/err_multiple_storage_storage_copies_fill_left.sol
index b2979804..902d8b98 100644
--- a/test/libsolidity/syntaxTests/tupleAssignments/warn_multiple_storage_storage_copies_fill_left.sol
+++ b/test/libsolidity/syntaxTests/tupleAssignments/err_multiple_storage_storage_copies_fill_left.sol
@@ -6,5 +6,5 @@ contract C {
 	}
 }
 // ----
+// TypeError: (89-101): Type tuple(int_const 1,int_const 2,struct C.S storage ref,struct C.S storage ref) is not implicitly convertible to expected type tuple(,struct C.S storage ref,struct C.S storage ref).
 // Warning: (79-101): This assignment performs two copies to storage. Since storage copies do not first copy to a temporary location, one of them might be overwritten before the second is executed and thus may have unexpected effects. It is safer to perform the copies separately or assign to storage pointers first.
-// Warning: (79-101): Different number of components on the left hand side (3) than on the right hand side (4).
diff --git a/test/libsolidity/syntaxTests/tupleAssignments/warn_multiple_storage_storage_copies_fill_right.sol b/test/libsolidity/syntaxTests/tupleAssignments/err_multiple_storage_storage_copies_fill_right.sol
index aa35d7d4..51556aab 100644
--- a/test/libsolidity/syntaxTests/tupleAssignments/warn_multiple_storage_storage_copies_fill_right.sol
+++ b/test/libsolidity/syntaxTests/tupleAssignments/err_multiple_storage_storage_copies_fill_right.sol
@@ -6,5 +6,5 @@ contract C {
 	}
 }
 // ----
+// TypeError: (90-102): Type tuple(struct C.S storage ref,struct C.S storage ref,int_const 1,int_const 2) is not implicitly convertible to expected type tuple(struct C.S storage ref,struct C.S storage ref,).
 // Warning: (79-102): This assignment performs two copies to storage. Since storage copies do not first copy to a temporary location, one of them might be overwritten before the second is executed and thus may have unexpected effects. It is safer to perform the copies separately or assign to storage pointers first.
-// Warning: (79-102): Different number of components on the left hand side (3) than on the right hand side (4).
diff --git a/test/libsolidity/syntaxTests/tupleAssignments/error_fill.sol b/test/libsolidity/syntaxTests/tupleAssignments/error_fill.sol
index 5b7f870b..ae722391 100644
--- a/test/libsolidity/syntaxTests/tupleAssignments/error_fill.sol
+++ b/test/libsolidity/syntaxTests/tupleAssignments/error_fill.sol
@@ -8,5 +8,5 @@ contract C {
 	}
 }
 // ----
-// TypeError: (126-136): Different number of components on the left hand side (2) than on the right hand side (3).
-// TypeError: (140-150): Different number of components on the left hand side (2) than on the right hand side (3).
+// TypeError: (133-136): Type tuple(uint256,uint256,bytes32) is not implicitly convertible to expected type tuple(uint256,).
+// TypeError: (147-150): Type tuple(uint256,uint256,bytes32) is not implicitly convertible to expected type tuple(,bytes32).
diff --git a/test/libsolidity/syntaxTests/tupleAssignments/nowarn_explicit_singleton_token_expression.sol b/test/libsolidity/syntaxTests/tupleAssignments/nowarn_explicit_singleton_token_expression.sol
deleted file mode 100644
index 3262781b..00000000
--- a/test/libsolidity/syntaxTests/tupleAssignments/nowarn_explicit_singleton_token_expression.sol
+++ /dev/null
@@ -1,8 +0,0 @@
-contract C {
-	function f() public pure {
-		uint a;
-		(a,) = (uint(1),);
-	}
-}
-// ----
-// Warning: (53-70): Different number of components on the left hand side (2) than on the right hand side (1).
diff --git a/test/libsolidity/syntaxTests/tupleAssignments/warn_fill_assignment.sol b/test/libsolidity/syntaxTests/tupleAssignments/warn_fill_assignment.sol
deleted file mode 100644
index a079a509..00000000
--- a/test/libsolidity/syntaxTests/tupleAssignments/warn_fill_assignment.sol
+++ /dev/null
@@ -1,11 +0,0 @@
-contract C {
-	function f() public pure returns (uint, uint, bytes32) {
-		uint a;
-		bytes32 b;
-		(a,) = f();
-		(,b) = f();
-	}
-}
-// ----
-// Warning: (96-106): Different number of components on the left hand side (2) than on the right hand side (3).
-// Warning: (110-120): Different number of components on the left hand side (2) than on the right hand side (3).