Merge pull request #4004 from ethereum/mulitVariableAssignment

Multi variable declarations

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.

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
 

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 {

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)

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.

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)

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

test/libsolidity/syntaxTests/multiVariableDeclaration/multiSingleVariableDeclaration.sol

@@ -0,0 +1,6 @@
+contract C {
+  function f() internal returns (uint) {
+    (uint a) = f();
+    a;
+  }
+} 

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.

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'

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.

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.

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

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.

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.