Merge pull request #4466 from ethereum/tupleNotEmpty

Disallow empty tuple components.

10 files changed, 27 insertions, 49 deletions

diff --git a/Changelog.md b/Changelog.md
index 1e92863b..65032511 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -33,6 +33,7 @@ Breaking Changes:
  * 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.
+ * Type Checker: Disallow empty tuple components. This was partly already the case in the experimental 0.5.0 mode.
  * Type Checker: Disallow specifying base constructor arguments multiple times in the same inheritance hierarchy. This was already the case in the experimental 0.5.0 mode.
  * Type Checker: Disallow uninitialized storage variables. This was already the case in the experimental 0.5.0 mode.
  * Type Checker: Only accept a single ``bytes`` type for ``.call()`` (and family), ``keccak256()``, ``sha256()`` and ``ripemd160()``.
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index a11b1879..81547bdf 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -1408,14 +1408,12 @@ bool TypeChecker::visit(TupleExpression const& _tuple)
 	}
 	else
 	{
-		bool const v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050);
 		bool isPure = true;
 		TypePointer inlineArrayType;
 
 		for (size_t i = 0; i < components.size(); ++i)
 		{
-			// Outside of an lvalue-context, the only situation where a component can be empty is (x,).
-			if (!components[i] && !(i == 1 && components.size() == 2))
+			if (!components[i])
 				m_errorReporter.fatalTypeError(_tuple.location(), "Tuple component cannot be empty.");
 			else if (components[i])
 			{
@@ -1427,10 +1425,7 @@ bool TypeChecker::visit(TupleExpression const& _tuple)
 					{
 						if (_tuple.isInlineArray())
 							m_errorReporter.fatalTypeError(components[i]->location(), "Array component cannot be empty.");
-						if (v050)
-							m_errorReporter.fatalTypeError(components[i]->location(), "Tuple component cannot be empty.");
-						else
-							m_errorReporter.warning(components[i]->location(), "Tuple component cannot be empty.");
+						m_errorReporter.typeError(components[i]->location(), "Tuple component cannot be empty.");
 					}
 
 				// Note: code generation will visit each of the expression even if they are not assigned from.
@@ -1468,11 +1463,7 @@ bool TypeChecker::visit(TupleExpression const& _tuple)
 			if (components.size() == 1)
 				_tuple.annotation().type = type(*components[0]);
 			else
-			{
-				if (components.size() == 2 && !components[1])
-					types.pop_back();
 				_tuple.annotation().type = make_shared<TupleType>(types);
-			}
 		}
 
 	}
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index fbc6a9c6..822b8192 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -7916,7 +7916,7 @@ BOOST_AUTO_TEST_CASE(string_tuples)
 				return (h(), "def");
 			}
 			function h() public returns (string) {
-				return ("abc",);
+				return ("abc");
 			}
 		}
 	)";
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/244_tuples.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/244_tuples.sol
index 3112f67a..95e8cf37 100644
--- a/test/libsolidity/syntaxTests/nameAndTypeResolution/244_tuples.sol
+++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/244_tuples.sol
@@ -1,13 +1,13 @@
 contract C {
     function f() public {
         uint a = (1);
-        (uint b,) = (uint8(1),);
+        (uint b,) = uint8(1);
         (uint c, uint d) = (uint32(1), 2 + a);
         (uint e,) = (uint64(1), 2, b);
         a;b;c;d;e;
     }
 }
 // ----
-// Warning: (69-92): Different number of components on the left hand side (2) than on the right hand side (1).
-// Warning: (149-178): Different number of components on the left hand side (2) than on the right hand side (3).
-// Warning: (17-204): Function state mutability can be restricted to pure
+// Warning: (69-89): Different number of components on the left hand side (2) than on the right hand side (1).
+// Warning: (146-175): Different number of components on the left hand side (2) than on the right hand side (3).
+// Warning: (17-201): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/parsing/tuples.sol b/test/libsolidity/syntaxTests/parsing/tuples.sol
index 8266c94f..ca2f9d6b 100644
--- a/test/libsolidity/syntaxTests/parsing/tuples.sol
+++ b/test/libsolidity/syntaxTests/parsing/tuples.sol
@@ -1,16 +1,16 @@
 contract C {
 	function f() public {
 		uint a = (1);
-		(uint b,) = (1,);
+		(uint b,) = 1;
 		(uint c, uint d) = (1, 2 + a);
 		(uint e,) = (1, 2, b);
 		(a) = 3;
 	}
 }
 // ----
-// Warning: (54-70): Different number of components on the left hand side (2) than on the right hand side (1).
-// Warning: (107-128): Different number of components on the left hand side (2) than on the right hand side (3).
-// Warning: (75-81): Unused local variable.
-// Warning: (83-89): Unused local variable.
-// Warning: (108-114): Unused local variable.
-// Warning: (14-143): Function state mutability can be restricted to pure
+// Warning: (54-67): Different number of components on the left hand side (2) than on the right hand side (1).
+// Warning: (104-125): Different number of components on the left hand side (2) than on the right hand side (3).
+// Warning: (72-78): Unused local variable.
+// Warning: (80-86): Unused local variable.
+// Warning: (105-111): Unused local variable.
+// Warning: (14-140): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/types/empty_tuple_function.sol b/test/libsolidity/syntaxTests/types/empty_tuple_function.sol
index 05b54442..ff31d440 100644
--- a/test/libsolidity/syntaxTests/types/empty_tuple_function.sol
+++ b/test/libsolidity/syntaxTests/types/empty_tuple_function.sol
@@ -8,5 +8,5 @@ contract C {
     }
 }
 // ----
-// Warning: (162-165): Tuple component cannot be empty.
-// Warning: (181-184): Tuple component cannot be empty.
+// TypeError: (162-165): Tuple component cannot be empty.
+// TypeError: (181-184): Tuple component cannot be empty.
diff --git a/test/libsolidity/syntaxTests/types/empty_tuple_function_050.sol b/test/libsolidity/syntaxTests/types/empty_tuple_function_050.sol
deleted file mode 100644
index c4b9e03f..00000000
--- a/test/libsolidity/syntaxTests/types/empty_tuple_function_050.sol
+++ /dev/null
@@ -1,11 +0,0 @@
-pragma experimental "v0.5.0";
-contract C {
-    function f() private pure {}
-    function a() public pure {
-        bool x = true;
-        bool y = true;
-        (x) ? (f(), y = false) : (f(), y = false);
-    }
-}
-// ----
-// TypeError: (168-171): Tuple component cannot be empty.
diff --git a/test/libsolidity/syntaxTests/types/empty_tuple_lvalue.sol b/test/libsolidity/syntaxTests/types/empty_tuple_lvalue.sol
index cba30c1b..3d252f0b 100644
--- a/test/libsolidity/syntaxTests/types/empty_tuple_lvalue.sol
+++ b/test/libsolidity/syntaxTests/types/empty_tuple_lvalue.sol
@@ -8,6 +8,6 @@ contract C {
     }
 }
 // ----
-// Warning: (146-149): Tuple component cannot be empty.
-// Warning: (151-154): Tuple component cannot be empty.
+// TypeError: (146-149): Tuple component cannot be empty.
+// TypeError: (151-154): Tuple component cannot be empty.
 // TypeError: (145-155): Type tuple(tuple(),tuple()) is not implicitly convertible to expected type tuple(uint256,uint256).
diff --git a/test/libsolidity/syntaxTests/types/empty_tuple_lvalue_050.sol b/test/libsolidity/syntaxTests/types/empty_tuple_lvalue_050.sol
deleted file mode 100644
index b0691778..00000000
--- a/test/libsolidity/syntaxTests/types/empty_tuple_lvalue_050.sol
+++ /dev/null
@@ -1,11 +0,0 @@
-pragma experimental "v0.5.0";
-contract C {
-    function f() private pure {}
-    function a() public {
-        uint x;
-        uint y;
-        (x, y) = (f(), f());
-    }
-}
-// ----
-// TypeError: (152-155): Tuple component cannot be empty.
diff --git a/test/libsolidity/syntaxTests/types/no_singleton_tuple.sol b/test/libsolidity/syntaxTests/types/no_singleton_tuple.sol
new file mode 100644
index 00000000..62a58f83
--- /dev/null
+++ b/test/libsolidity/syntaxTests/types/no_singleton_tuple.sol
@@ -0,0 +1,8 @@
+contract C {
+	function f() public pure {
+		uint a;
+		(a,) = (uint(1),);
+	}
+}
+// ----
+// TypeError: (60-70): Tuple component cannot be empty.