Merge pull request #4465 from ethereum/tupleAssignment

Disallow tuple assignment with mismatching number of components.

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.

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.
         }
     }
 

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)
 	{

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

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"(

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).

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).

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).

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).

test/libsolidity/syntaxTests/tupleAssignments/nowarn_explicit_singleton_token_expression.sol

@@ -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).

test/libsolidity/syntaxTests/tupleAssignments/warn_fill_assignment.sol

@@ -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).