Merge pull request #4004 from ethereum/mulitVariableAssignment

Multi variable declarations

15 files changed, 200 insertions, 23 deletions

diff --git a/Changelog.md b/Changelog.md
index d6f54070..72c27f8e 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -7,6 +7,7 @@ Features:
  * Control Flow Graph: Add Control Flow Graph as analysis structure.
  * Control Flow Graph: Warn about returning uninitialized storage pointers.
  * Gas Estimator: Only explore paths with higher gas costs. This reduces accuracy but greatly improves the speed of gas estimation.
+ * General: Allow multiple variables to be declared as part of a tuple assignment, e.g. ``(uint a, uint b) = ...``.
  * Optimizer: Remove unnecessary masking of the result of known short instructions (``ADDRESS``, ``CALLER``, ``ORIGIN`` and ``COINBASE``).
  * Parser: Display nicer error messages by showing the actual tokens and not internal names.
  * Parser: Use the entire location of the token instead of only its starting position as source location for parser errors.
diff --git a/docs/control-structures.rst b/docs/control-structures.rst
index f18e1e10..7849d15a 100644
--- a/docs/control-structures.rst
+++ b/docs/control-structures.rst
@@ -272,9 +272,12 @@ Assignment
 Destructuring Assignments and Returning Multiple Values
 -------------------------------------------------------
 
-Solidity internally allows tuple types, i.e. a list of objects of potentially different types whose size is a constant at compile-time. Those tuples can be used to return multiple values at the same time and also assign them to multiple variables (or LValues in general) at the same time::
+Solidity internally allows tuple types, i.e. a list of objects of potentially different types whose size is a constant at compile-time. Those tuples can be used to return multiple values at the same time.
+These can then either be assigned to newly declared variables or to pre-existing variables (or LValues in general):
 
-    pragma solidity ^0.4.16;
+::
+
+    pragma solidity >0.4.23 <0.5.0;
 
     contract C {
         uint[] data;
@@ -284,12 +287,8 @@ Solidity internally allows tuple types, i.e. a list of objects of potentially di
         }
 
         function g() public {
-            // Variables declared with type
-            uint x;
-            bool b;
-            uint y;
-            // Tuple values can be assigned to these pre-existing variables
-            (x, b, y) = f();
+            // Variables declared with type and assigned from the returned tuple.
+            (uint x, bool b, uint y) = f();
             // Common trick to swap values -- does not work for non-value storage types.
             (x, y) = (y, x);
             // Components can be left out (also for variable declarations).
@@ -330,7 +329,9 @@ A variable declared anywhere within a function will be in scope for the *entire
 (this will change soon, see below).
 This happens because Solidity inherits its scoping rules from JavaScript.
 This is in contrast to many languages where variables are only scoped where they are declared until the end of the semantic block.
-As a result, the following code is illegal and cause the compiler to throw an error, ``Identifier already declared``::
+As a result, the following code is illegal and cause the compiler to throw an error, ``Identifier already declared``:
+
+::
 
     // This will not compile
 
diff --git a/docs/frequently-asked-questions.rst b/docs/frequently-asked-questions.rst
index 6a2fe685..ca5a1aee 100644
--- a/docs/frequently-asked-questions.rst
+++ b/docs/frequently-asked-questions.rst
@@ -203,7 +203,7 @@ situation.
 
 If you do not want to throw, you can return a pair::
 
-    pragma solidity ^0.4.16;
+    pragma solidity >0.4.23 <0.5.0;
 
     contract C {
         uint[] counters;
@@ -219,7 +219,7 @@ If you do not want to throw, you can return a pair::
         }
 
         function checkCounter(uint index) public view {
-            var (counter, error) = getCounter(index);
+            (uint counter, bool error) = getCounter(index);
             if (error) {
                 // ...
             } else {
diff --git a/docs/grammar.txt b/docs/grammar.txt
index 565db9a4..0dda4f49 100644
--- a/docs/grammar.txt
+++ b/docs/grammar.txt
@@ -78,7 +78,7 @@ Break = 'break'
 Return = 'return' Expression?
 Throw = 'throw'
 EmitStatement = 'emit' FunctionCall
-VariableDefinition = ('var' IdentifierList | VariableDeclaration) ( '=' Expression )?
+VariableDefinition = ('var' IdentifierList | VariableDeclaration | '(' VariableDeclaration? (',' VariableDeclaration? )* ')' ) ( '=' Expression )?
 IdentifierList = '(' ( Identifier? ',' )* Identifier? ')'
 
 // Precedence by order (see github.com/ethereum/solidity/pull/732)
diff --git a/docs/solidity-by-example.rst b/docs/solidity-by-example.rst
index f6038f7d..2b3d4b48 100644
--- a/docs/solidity-by-example.rst
+++ b/docs/solidity-by-example.rst
@@ -388,7 +388,7 @@ high or low invalid bids.
 
 ::
 
-    pragma solidity ^0.4.22;
+    pragma solidity >0.4.23 <0.5.0;
 
     contract BlindAuction {
         struct Bid {
@@ -467,8 +467,8 @@ high or low invalid bids.
 
             uint refund;
             for (uint i = 0; i < length; i++) {
-                var bid = bids[msg.sender][i];
-                var (value, fake, secret) =
+                Bid storage bid = bids[msg.sender][i];
+                (uint value, bool fake, bytes32 secret) =
                         (_values[i], _fake[i], _secret[i]);
                 if (bid.blindedBid != keccak256(value, fake, secret)) {
                     // Bid was not actually revealed.
diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp
index d1be13a5..e2e1eebc 100644
--- a/libsolidity/parsing/Parser.cpp
+++ b/libsolidity/parsing/Parser.cpp
@@ -54,6 +54,7 @@ public:
 	template <class NodeType, typename... Args>
 	ASTPointer<NodeType> createNode(Args&& ... _args)
 	{
+		solAssert(m_location.sourceName, "");
 		if (m_location.end < 0)
 			markEndPosition();
 		return make_shared<NodeType>(m_location, forward<Args>(_args)...);
@@ -1086,15 +1087,79 @@ ASTPointer<Statement> Parser::parseSimpleStatement(ASTPointer<ASTString> const&
 	LookAheadInfo statementType;
 	IndexAccessedPath iap;
 
-	tie(statementType, iap) = tryParseIndexAccessedPath();
-	switch (statementType)
+	if (m_scanner->currentToken() == Token::LParen)
 	{
-	case LookAheadInfo::VariableDeclaration:
-		return parseVariableDeclarationStatement(_docString, typeNameFromIndexAccessStructure(iap));
-	case LookAheadInfo::Expression:
-		return parseExpressionStatement(_docString, expressionFromIndexAccessStructure(iap));
-	default:
-		solAssert(false, "");
+		ASTNodeFactory nodeFactory(*this);
+		size_t emptyComponents = 0;
+		// First consume all empty components.
+		expectToken(Token::LParen);
+		while (m_scanner->currentToken() == Token::Comma)
+		{
+			m_scanner->next();
+			emptyComponents++;
+		}
+
+		// Now see whether we have a variable declaration or an expression.
+		tie(statementType, iap) = tryParseIndexAccessedPath();
+		switch (statementType)
+		{
+		case LookAheadInfo::VariableDeclaration:
+		{
+			vector<ASTPointer<VariableDeclaration>> variables;
+			ASTPointer<Expression> value;
+			// We have already parsed something like `(,,,,a.b.c[2][3]`
+			VarDeclParserOptions options;
+			options.allowLocationSpecifier = true;
+			variables = vector<ASTPointer<VariableDeclaration>>(emptyComponents, nullptr);
+			variables.push_back(parseVariableDeclaration(options, typeNameFromIndexAccessStructure(iap)));
+
+			while (m_scanner->currentToken() != Token::RParen)
+			{
+				expectToken(Token::Comma);
+				if (m_scanner->currentToken() == Token::Comma || m_scanner->currentToken() == Token::RParen)
+					variables.push_back(nullptr);
+				else
+					variables.push_back(parseVariableDeclaration(options));
+			}
+			expectToken(Token::RParen);
+			expectToken(Token::Assign);
+			value = parseExpression();
+			nodeFactory.setEndPositionFromNode(value);
+			return nodeFactory.createNode<VariableDeclarationStatement>(_docString, variables, value);
+		}
+		case LookAheadInfo::Expression:
+		{
+			// Complete parsing the expression in the current component.
+			vector<ASTPointer<Expression>> components(emptyComponents, nullptr);
+			components.push_back(parseExpression(expressionFromIndexAccessStructure(iap)));
+			while (m_scanner->currentToken() != Token::RParen)
+			{
+				expectToken(Token::Comma);
+				if (m_scanner->currentToken() == Token::Comma || m_scanner->currentToken() == Token::RParen)
+					components.push_back(ASTPointer<Expression>());
+				else
+					components.push_back(parseExpression());
+			}
+			nodeFactory.markEndPosition();
+			expectToken(Token::RParen);
+			return parseExpressionStatement(_docString, nodeFactory.createNode<TupleExpression>(components, false));
+		}
+		default:
+			solAssert(false, "");
+		}
+	}
+	else
+	{
+		tie(statementType, iap) = tryParseIndexAccessedPath();
+		switch (statementType)
+		{
+		case LookAheadInfo::VariableDeclaration:
+			return parseVariableDeclarationStatement(_docString, typeNameFromIndexAccessStructure(iap));
+		case LookAheadInfo::Expression:
+			return parseExpressionStatement(_docString, expressionFromIndexAccessStructure(iap));
+		default:
+			solAssert(false, "");
+		}
 	}
 }
 
@@ -1144,6 +1209,9 @@ ASTPointer<VariableDeclarationStatement> Parser::parseVariableDeclarationStateme
 	ASTPointer<TypeName> const& _lookAheadArrayType
 )
 {
+	// This does not parse multi variable declaration statements starting directly with
+	// `(`, they are parsed in parseSimpleStatement, because they are hard to distinguish
+	// from tuple expressions.
 	RecursionGuard recursionGuard(*this);
 	ASTNodeFactory nodeFactory(*this);
 	if (_lookAheadArrayType)
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 2db2aadd..42f69099 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -7610,6 +7610,33 @@ BOOST_AUTO_TEST_CASE(multi_variable_declaration)
 	ABI_CHECK(callContractFunction("f()", encodeArgs()), encodeArgs(true));
 }
 
+BOOST_AUTO_TEST_CASE(typed_multi_variable_declaration)
+{
+	char const* sourceCode = R"(
+		contract C {
+			struct S { uint x; }
+			S s;
+			function g() internal returns (uint, S storage, uint) {
+				s.x = 7;
+				return (1, s, 2);
+			}
+			function f() returns (bool) {
+				(uint x1, S storage y1, uint z1) = g();
+				if (x1 != 1 || y1.x != 7 || z1 != 2) return false;
+				(, S storage y2,) = g();
+				if (y2.x != 7) return false;
+				(uint x2,,) = g();
+				if (x2 != 1) return false;
+				(,,uint z2) = g();
+				if (z2 != 2) return false;
+				return true;
+			}
+		}
+	)";
+	compileAndRun(sourceCode);
+	ABI_CHECK(callContractFunction("f()", encodeArgs()), encodeArgs(true));
+}
+
 BOOST_AUTO_TEST_CASE(tuples)
 {
 	char const* sourceCode = R"(
diff --git a/test/libsolidity/syntaxTests/multiVariableDeclaration/multiSingleVariableDeclaration.sol b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiSingleVariableDeclaration.sol
new file mode 100644
index 00000000..182ba072
--- /dev/null
+++ b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiSingleVariableDeclaration.sol
@@ -0,0 +1,6 @@
+contract C {
+  function f() internal returns (uint) {
+    (uint a) = f();
+    a;
+  }
+} 
diff --git a/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationComplex.sol b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationComplex.sol
new file mode 100644
index 00000000..a3ce6a74
--- /dev/null
+++ b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationComplex.sol
@@ -0,0 +1,11 @@
+contract D {
+  struct S { uint a; uint b; }
+}
+contract C {
+  function f() internal returns (uint, uint, uint, D.S[20] storage, uint) {
+    (,,,D.S[10*2] storage x,) = f();
+    x;
+  }
+} 
+// ----
+// Warning: (110-117): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
diff --git a/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationInvalid.sol b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationInvalid.sol
new file mode 100644
index 00000000..c8686ae8
--- /dev/null
+++ b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationInvalid.sol
@@ -0,0 +1,8 @@
+contract C {
+  function f() internal returns (uint, uint, uint, uint) {
+    var (uint a, uint b,,) = f();
+    a; b;
+  }
+} 
+// ----
+// ParserError: (81-85): Expected identifier but got 'uint'
diff --git a/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationInvalidType.sol b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationInvalidType.sol
new file mode 100644
index 00000000..2b765837
--- /dev/null
+++ b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationInvalidType.sol
@@ -0,0 +1,9 @@
+contract C {
+  function f() internal returns (string memory, uint, uint, uint) {
+    (uint a, string memory b,,) = f();
+    a; b;
+  }
+} 
+// ----
+// TypeError: (85-118): Type string memory is not implicitly convertible to expected type uint256.
+// TypeError: (85-118): Type uint256 is not implicitly convertible to expected type string memory.
diff --git a/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationScoping.sol b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationScoping.sol
new file mode 100644
index 00000000..3ba85f69
--- /dev/null
+++ b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationScoping.sol
@@ -0,0 +1,12 @@
+pragma experimental "v0.5.0";
+
+contract C {
+  function f() internal {
+    {
+      (uint a, uint b, uint c) = (1, 2, 3);
+    }
+    a;
+  }
+} 
+// ----
+// DeclarationError: (130-131): Undeclared identifier.
diff --git a/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationScoping2.sol b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationScoping2.sol
new file mode 100644
index 00000000..e21181de
--- /dev/null
+++ b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationScoping2.sol
@@ -0,0 +1,13 @@
+pragma experimental "v0.5.0";
+
+contract C {
+  function f() internal {
+    {
+      (uint a, uint b, uint c) = (a, b, c);
+    }
+  }
+} 
+// ----
+// DeclarationError: (110-111): Undeclared identifier. Did you mean "a"?
+// DeclarationError: (113-114): Undeclared identifier. Did you mean "b"?
+// DeclarationError: (116-117): Undeclared identifier. Did you mean "c"?
diff --git a/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationSimple.sol b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationSimple.sol
new file mode 100644
index 00000000..8e06322c
--- /dev/null
+++ b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationSimple.sol
@@ -0,0 +1,12 @@
+contract C {
+  function f() internal returns (uint, uint, uint, uint) {
+    (uint a, uint b,,) = f();
+    a; b;
+  }
+  function g() internal returns (bytes memory, string storage) {
+    (bytes memory a, string storage b) = g();
+    a; b;
+  }
+} 
+// ----
+// Warning: (163-169): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
diff --git a/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationThatIsExpression.sol b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationThatIsExpression.sol
new file mode 100644
index 00000000..8ae0eaac
--- /dev/null
+++ b/test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationThatIsExpression.sol
@@ -0,0 +1,9 @@
+contract C {
+  struct S { function() returns (S storage)[] x; }
+  S s;
+  function f() internal pure returns (uint, uint, uint, S storage, uint, uint) {
+    (,,,s.x[2](),,) = f();
+  }
+} 
+// ----
+// TypeError: (160-168): Expression has to be an lvalue.