From 67d208d144a179e52e1aab3fbd1bd67fe20176b7 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 26 Apr 2018 10:43:11 +0200 Subject: Parse multi variable declaration statement. --- Changelog.md | 1 + libsolidity/parsing/Parser.cpp | 83 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 76 insertions(+), 8 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/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index d1be13a5..01c1fa99 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -1086,15 +1086,79 @@ ASTPointer Parser::parseSimpleStatement(ASTPointer 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> variables; + ASTPointer value; + // We have already parsed something like `(,,,,a.b.c[2][3]` + VarDeclParserOptions options; + options.allowLocationSpecifier = true; + variables = vector>(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(_docString, variables, value); + } + case LookAheadInfo::Expression: + { + // Complete parsing the expression in the current component. + vector> 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()); + else + components.push_back(parseExpression()); + } + nodeFactory.markEndPosition(); + expectToken(Token::RParen); + return parseExpressionStatement(_docString, nodeFactory.createNode(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 +1208,9 @@ ASTPointer Parser::parseVariableDeclarationStateme ASTPointer 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) -- cgit v1.2.3 From c781baf7336af55abc33e1b63e6fc99a7e555d78 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 27 Apr 2018 15:44:41 +0200 Subject: Add tests for multi variable declaration statement. --- test/libsolidity/SolidityEndToEndTest.cpp | 27 ++++++++++++++++++++++ .../multiSingleVariableDeclaration.sol | 6 +++++ .../multiVariableDeclarationComplex.sol | 11 +++++++++ .../multiVariableDeclarationInvalid.sol | 8 +++++++ .../multiVariableDeclarationInvalidType.sol | 9 ++++++++ .../multiVariableDeclarationScoping.sol | 12 ++++++++++ .../multiVariableDeclarationScoping2.sol | 13 +++++++++++ .../multiVariableDeclarationSimple.sol | 12 ++++++++++ .../multiVariableDeclarationThatIsExpression.sol | 9 ++++++++ 9 files changed, 107 insertions(+) create mode 100644 test/libsolidity/syntaxTests/multiVariableDeclaration/multiSingleVariableDeclaration.sol create mode 100644 test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationComplex.sol create mode 100644 test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationInvalid.sol create mode 100644 test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationInvalidType.sol create mode 100644 test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationScoping.sol create mode 100644 test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationScoping2.sol create mode 100644 test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationSimple.sol create mode 100644 test/libsolidity/syntaxTests/multiVariableDeclaration/multiVariableDeclarationThatIsExpression.sol 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. -- cgit v1.2.3 From 6c8f78fb8ff99c5d28669fa7383a25b8c8523915 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 30 Apr 2018 13:30:09 +0200 Subject: Update documentation for multi variable declaration statement. --- docs/control-structures.rst | 19 ++++++++++--------- docs/frequently-asked-questions.rst | 4 ++-- docs/grammar.txt | 2 +- docs/solidity-by-example.rst | 6 +++--- 4 files changed, 16 insertions(+), 15 deletions(-) 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. -- cgit v1.2.3 From 3ca6738114f9ae410c7488a1b1d66eeec92833c6 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 30 Apr 2018 15:26:25 +0200 Subject: Add assert about source location. --- libsolidity/parsing/Parser.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index 01c1fa99..e2e1eebc 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -54,6 +54,7 @@ public: template ASTPointer createNode(Args&& ... _args) { + solAssert(m_location.sourceName, ""); if (m_location.end < 0) markEndPosition(); return make_shared(m_location, forward(_args)...); -- cgit v1.2.3