Remove remaining instances of ``fillRight`` left over from tuple wildcards assignments.

7 files changed, 26 insertions, 41 deletions

diff --git a/Changelog.md b/Changelog.md
index 0790c0ca..1974ffb2 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -90,6 +90,7 @@ Bugfixes:
  * Type Checker: Disallow assignments to mappings within tuple assignments as well.
  * Type Checker: Allow assignments to local variables of mapping types.
  * Type Checker: Consider fixed size arrays when checking for recursive structs.
+ * Type Checker: Fix crashes in erroneous tuple assignments in which the type of the right hand side cannot be determined.
  * Type Checker: Report error when using structs in events without experimental ABIEncoderV2. This used to crash or log the wrong values.
  * Type System: Allow arbitrary exponents for literals with a mantissa of zero.
 
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index 38331a43..bd3187ae 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -498,7 +498,12 @@ void TypeChecker::checkDoubleStorageAssignment(Assignment const& _assignment)
 	TupleType const& lhs = dynamic_cast<TupleType const&>(*type(_assignment.leftHandSide()));
 	TupleType const& rhs = dynamic_cast<TupleType const&>(*type(_assignment.rightHandSide()));
 
-	bool fillRight = !lhs.components().empty() && (!lhs.components().back() || lhs.components().front());
+	if (lhs.components().size() != rhs.components().size())
+	{
+		solAssert(m_errorReporter.hasErrors(), "");
+		return;
+	}
+
 	size_t storageToStorageCopies = 0;
 	size_t toStorageCopies = 0;
 	for (size_t i = 0; i < lhs.components().size(); ++i)
@@ -506,10 +511,8 @@ void TypeChecker::checkDoubleStorageAssignment(Assignment const& _assignment)
 		ReferenceType const* ref = dynamic_cast<ReferenceType const*>(lhs.components()[i].get());
 		if (!ref || !ref->dataStoredIn(DataLocation::Storage) || ref->isPointer())
 			continue;
-		size_t rhsPos = fillRight ? i : rhs.components().size() - (lhs.components().size() - i);
-		solAssert(rhsPos < rhs.components().size(), "");
 		toStorageCopies++;
-		if (rhs.components()[rhsPos]->dataStoredIn(DataLocation::Storage))
+		if (rhs.components()[i]->dataStoredIn(DataLocation::Storage))
 			storageToStorageCopies++;
 	}
 	if (storageToStorageCopies >= 1 && toStorageCopies >= 2)
diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp
index 3eccc6d4..2b5f2187 100644
--- a/libsolidity/ast/Types.cpp
+++ b/libsolidity/ast/Types.cpp
@@ -2293,16 +2293,14 @@ TypePointer TupleType::closestTemporaryType(TypePointer const& _targetType) cons
 {
 	solAssert(!!_targetType, "");
 	TypePointers const& targetComponents = dynamic_cast<TupleType const&>(*_targetType).components();
-	bool fillRight = !targetComponents.empty() && (!targetComponents.back() || targetComponents.front());
+	solAssert(components().size() == targetComponents.size(), "");
 	TypePointers tempComponents(targetComponents.size());
-	for (size_t i = 0; i < min(targetComponents.size(), components().size()); ++i)
+	for (size_t i = 0; i < targetComponents.size(); ++i)
 	{
-		size_t si = fillRight ? i : components().size() - i - 1;
-		size_t ti = fillRight ? i : targetComponents.size() - i - 1;
-		if (components()[si] && targetComponents[ti])
+		if (components()[i] && targetComponents[i])
 		{
-			tempComponents[ti] = components()[si]->closestTemporaryType(targetComponents[ti]);
-			solAssert(tempComponents[ti], "");
+			tempComponents[i] = components()[i]->closestTemporaryType(targetComponents[i]);
+			solAssert(tempComponents[i], "");
 		}
 	}
 	return make_shared<TupleType>(tempComponents);
diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp
index bd3ec8f5..b30851fb 100644
--- a/libsolidity/codegen/CompilerUtils.cpp
+++ b/libsolidity/codegen/CompilerUtils.cpp
@@ -947,20 +947,12 @@ void CompilerUtils::convertType(
 	{
 		TupleType const& sourceTuple = dynamic_cast<TupleType const&>(_typeOnStack);
 		TupleType const& targetTuple = dynamic_cast<TupleType const&>(_targetType);
-		// fillRight: remove excess values at right side, !fillRight: remove eccess values at left side
-		bool fillRight = !targetTuple.components().empty() && (
-			!targetTuple.components().back() ||
-			targetTuple.components().front()
-		);
+		solAssert(targetTuple.components().size() == sourceTuple.components().size(), "");
 		unsigned depth = sourceTuple.sizeOnStack();
 		for (size_t i = 0; i < sourceTuple.components().size(); ++i)
 		{
 			TypePointer sourceType = sourceTuple.components()[i];
-			TypePointer targetType;
-			if (fillRight && i < targetTuple.components().size())
-				targetType = targetTuple.components()[i];
-			else if (!fillRight && targetTuple.components().size() + i >= sourceTuple.components().size())
-				targetType = targetTuple.components()[targetTuple.components().size() - (sourceTuple.components().size() - i)];
+			TypePointer targetType = targetTuple.components()[i];
 			if (!sourceType)
 			{
 				solAssert(!targetType, "");
diff --git a/test/libsolidity/syntaxTests/tupleAssignments/double_storage_crash.sol b/test/libsolidity/syntaxTests/tupleAssignments/double_storage_crash.sol
new file mode 100644
index 00000000..3cff3a9a
--- /dev/null
+++ b/test/libsolidity/syntaxTests/tupleAssignments/double_storage_crash.sol
@@ -0,0 +1,11 @@
+// This used to crash in certain compiler versions.
+contract CrashContract {
+       struct S { uint a; }
+       S x;
+       function f() public {
+               (x, x) = 1(x, x);
+       }
+}
+// ----
+// TypeError: (170-177): Type is not callable
+// TypeError: (170-177): Type tuple() is not implicitly convertible to expected type tuple(struct CrashContract.S storage ref,struct CrashContract.S storage ref).
diff --git a/test/libsolidity/syntaxTests/tupleAssignments/err_multiple_storage_storage_copies_fill_left.sol b/test/libsolidity/syntaxTests/tupleAssignments/err_multiple_storage_storage_copies_fill_left.sol
deleted file mode 100644
index 902d8b98..00000000
--- a/test/libsolidity/syntaxTests/tupleAssignments/err_multiple_storage_storage_copies_fill_left.sol
+++ /dev/null
@@ -1,10 +0,0 @@
-contract C {
-	struct S { uint a; uint b; }
-	S x; S y;
-	function f() public {
-		(,x, y) = (1, 2, y, x);
-	}
-}
-// ----
-// 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.
diff --git a/test/libsolidity/syntaxTests/tupleAssignments/err_multiple_storage_storage_copies_fill_right.sol b/test/libsolidity/syntaxTests/tupleAssignments/err_multiple_storage_storage_copies_fill_right.sol
deleted file mode 100644
index 51556aab..00000000
--- a/test/libsolidity/syntaxTests/tupleAssignments/err_multiple_storage_storage_copies_fill_right.sol
+++ /dev/null
@@ -1,10 +0,0 @@
-contract C {
-	struct S { uint a; uint b; }
-	S x; S y;
-	function f() public {
-		(x, y, ) = (y, x, 1, 2);
-	}
-}
-// ----
-// 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.