From 02984b8de11a9dc6ab88788bfae82530b073f1f6 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 5 Sep 2016 14:54:50 +0200 Subject: Require ";" after "_" --- Changelog.md | 1 + docs/common-patterns.rst | 14 ++++----- docs/contracts.rst | 6 ++-- docs/solidity-by-example.rst | 4 +-- libsolidity/parsing/Parser.cpp | 2 +- test/contracts/AuctionRegistrar.cpp | 2 +- test/contracts/FixedFeeRegistrar.cpp | 2 +- test/contracts/Wallet.cpp | 6 ++-- test/libsolidity/ASTJSON.cpp | 10 +++---- test/libsolidity/SolidityEndToEndTest.cpp | 34 +++++++++++----------- test/libsolidity/SolidityNameAndTypeResolution.cpp | 26 ++++++++--------- test/libsolidity/SolidityParser.cpp | 16 +++++++--- 12 files changed, 66 insertions(+), 57 deletions(-) diff --git a/Changelog.md b/Changelog.md index 25890890..18ca861c 100644 --- a/Changelog.md +++ b/Changelog.md @@ -17,6 +17,7 @@ Breaking Changes: * Function call throws if target contract does not have code * Modifiers are required to contain ``_`` (use ``if (false) _`` as a workaround if needed). * Modifiers: return does not skip part in modifier after ``_`` + * Placeholder statement `_` in modifier now requires explicit `;`. * ``ecrecover`` now returns zero if the input is malformed (it previously returned garbage) * Removed ``--interface`` (Solidity interface) output option * JSON AST: General cleanup, renamed many nodes to match their C++ names. diff --git a/docs/common-patterns.rst b/docs/common-patterns.rst index 531a94ad..0894aae2 100644 --- a/docs/common-patterns.rst +++ b/docs/common-patterns.rst @@ -148,10 +148,10 @@ restrictions highly readable. { if (msg.sender != _account) throw; - // Do not forget the "_"! It will + // Do not forget the "_;"! It will // be replaced by the actual function // body when the modifier is invoked. - _ + _; } /// Make `_newOwner` the new owner of this @@ -164,7 +164,7 @@ restrictions highly readable. modifier onlyAfter(uint _time) { if (now < _time) throw; - _ + _; } /// Erase ownership information. @@ -187,7 +187,7 @@ restrictions highly readable. modifier costs(uint _amount) { if (msg.value < _amount) throw; - _ + _; if (msg.value > _amount) msg.sender.send(msg.value - _amount); } @@ -286,7 +286,7 @@ function finishes. modifier atStage(Stages _stage) { if (stage != _stage) throw; - _ + _; } function nextStage() internal { @@ -304,7 +304,7 @@ function finishes. now >= creationTime + 12 days) nextStage(); // The other stages transition by transaction - _ + _; } // Order of the modifiers matters here! @@ -328,7 +328,7 @@ function finishes. // automatically. modifier transitionNext() { - _ + _; nextStage(); } diff --git a/docs/contracts.rst b/docs/contracts.rst index a22a3544..dfa79e79 100644 --- a/docs/contracts.rst +++ b/docs/contracts.rst @@ -321,7 +321,7 @@ inheritable properties of contracts and may be overridden by derived contracts. modifier onlyOwner { if (msg.sender != owner) throw; - _ + _; } } @@ -341,7 +341,7 @@ inheritable properties of contracts and may be overridden by derived contracts. // Modifiers can receive arguments: modifier costs(uint price) { if (msg.value >= price) { - _ + _; } } } @@ -367,7 +367,7 @@ inheritable properties of contracts and may be overridden by derived contracts. modifier noReentrancy() { if (locked) throw; locked = true; - _ + _; locked = false; } diff --git a/docs/solidity-by-example.rst b/docs/solidity-by-example.rst index 8e23dafd..b8fa4a73 100644 --- a/docs/solidity-by-example.rst +++ b/docs/solidity-by-example.rst @@ -403,8 +403,8 @@ high or low invalid bids. /// functions. `onlyBefore` is applied to `bid` below: /// The new function body is the modifier's body where /// `_` is replaced by the old function body. - modifier onlyBefore(uint _time) { if (now >= _time) throw; _ } - modifier onlyAfter(uint _time) { if (now <= _time) throw; _ } + modifier onlyBefore(uint _time) { if (now >= _time) throw; _; } + modifier onlyAfter(uint _time) { if (now <= _time) throw; _; } function BlindAuction( uint _biddingTime, diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index b2f4a156..1bffa5d7 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -753,7 +753,7 @@ ASTPointer Parser::parseStatement() { statement = ASTNodeFactory(*this).createNode(docString); m_scanner->next(); - return statement; + break; } // fall-through default: diff --git a/test/contracts/AuctionRegistrar.cpp b/test/contracts/AuctionRegistrar.cpp index 681caa26..9161f2ce 100644 --- a/test/contracts/AuctionRegistrar.cpp +++ b/test/contracts/AuctionRegistrar.cpp @@ -158,7 +158,7 @@ contract GlobalRegistrar is Registrar, AuctionSystem { return bytes(_name).length < c_freeBytes; } - modifier onlyrecordowner(string _name) { if (m_toRecord[_name].owner == msg.sender) _ } + modifier onlyrecordowner(string _name) { if (m_toRecord[_name].owner == msg.sender) _; } function transfer(string _name, address _newOwner) onlyrecordowner(_name) { m_toRecord[_name].owner = _newOwner; diff --git a/test/contracts/FixedFeeRegistrar.cpp b/test/contracts/FixedFeeRegistrar.cpp index fd0861f7..c37873cf 100644 --- a/test/contracts/FixedFeeRegistrar.cpp +++ b/test/contracts/FixedFeeRegistrar.cpp @@ -71,7 +71,7 @@ contract FixedFeeRegistrar is Registrar { address owner; } - modifier onlyrecordowner(string _name) { if (m_record(_name).owner == msg.sender) _ } + modifier onlyrecordowner(string _name) { if (m_record(_name).owner == msg.sender) _; } function reserve(string _name) { Record rec = m_record(_name); diff --git a/test/contracts/Wallet.cpp b/test/contracts/Wallet.cpp index 55c2e1af..d3e1b8d7 100644 --- a/test/contracts/Wallet.cpp +++ b/test/contracts/Wallet.cpp @@ -86,14 +86,14 @@ contract multiowned { // simple single-sig function modifier. modifier onlyowner { if (isOwner(msg.sender)) - _ + _; } // multi-sig function modifier: the operation must have an intrinsic hash in order // that later attempts can be realised as the same underlying operation and // thus count as confirmations. modifier onlymanyowners(bytes32 _operation) { if (confirmAndCheck(_operation)) - _ + _; } // METHODS @@ -281,7 +281,7 @@ contract daylimit is multiowned { // simple modifier for daily limit. modifier limitedDaily(uint _value) { if (underLimit(_value)) - _ + _; } // METHODS diff --git a/test/libsolidity/ASTJSON.cpp b/test/libsolidity/ASTJSON.cpp index ec60b668..882496d9 100644 --- a/test/libsolidity/ASTJSON.cpp +++ b/test/libsolidity/ASTJSON.cpp @@ -128,7 +128,7 @@ BOOST_AUTO_TEST_CASE(enum_value) BOOST_AUTO_TEST_CASE(modifier_definition) { CompilerStack c; - c.addSource("a", "contract C { modifier M(uint i) { _ } function F() M(1) {} }"); + c.addSource("a", "contract C { modifier M(uint i) { _; } function F() M(1) {} }"); c.parse(); map sourceIndices; sourceIndices["a"] = 1; @@ -136,20 +136,20 @@ BOOST_AUTO_TEST_CASE(modifier_definition) Json::Value modifier = astJson["children"][0]["children"][0]; BOOST_CHECK_EQUAL(modifier["name"], "ModifierDefinition"); BOOST_CHECK_EQUAL(modifier["attributes"]["name"], "M"); - BOOST_CHECK_EQUAL(modifier["src"], "13:24:1"); + BOOST_CHECK_EQUAL(modifier["src"], "13:25:1"); } BOOST_AUTO_TEST_CASE(modifier_invocation) { CompilerStack c; - c.addSource("a", "contract C { modifier M(uint i) { _ } function F() M(1) {} }"); + c.addSource("a", "contract C { modifier M(uint i) { _; } function F() M(1) {} }"); c.parse(); map sourceIndices; sourceIndices["a"] = 1; Json::Value astJson = ASTJsonConverter(c.ast("a"), sourceIndices).json(); Json::Value modifier = astJson["children"][0]["children"][1]["children"][2]; BOOST_CHECK_EQUAL(modifier["name"], "ModifierInvocation"); - BOOST_CHECK_EQUAL(modifier["src"], "51:4:1"); + BOOST_CHECK_EQUAL(modifier["src"], "52:4:1"); BOOST_CHECK_EQUAL(modifier["children"][0]["attributes"]["type"], "modifier (uint256)"); BOOST_CHECK_EQUAL(modifier["children"][0]["attributes"]["value"], "M"); BOOST_CHECK_EQUAL(modifier["children"][1]["attributes"]["value"], "1"); @@ -185,7 +185,7 @@ BOOST_AUTO_TEST_CASE(array_type_name) BOOST_AUTO_TEST_CASE(placeholder_statement) { CompilerStack c; - c.addSource("a", "contract C { modifier M { _ } }"); + c.addSource("a", "contract C { modifier M { _; } }"); c.parse(); map sourceIndices; sourceIndices["a"] = 1; diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 35b7078d..f3fdef15 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -2353,7 +2353,7 @@ BOOST_AUTO_TEST_CASE(function_modifier) char const* sourceCode = R"( contract C { function getOne() nonFree returns (uint r) { return 1; } - modifier nonFree { if (msg.value > 0) _ } + modifier nonFree { if (msg.value > 0) _; } } )"; compileAndRun(sourceCode); @@ -2365,8 +2365,8 @@ BOOST_AUTO_TEST_CASE(function_modifier_local_variables) { char const* sourceCode = R"( contract C { - modifier mod1 { var a = 1; var b = 2; _ } - modifier mod2(bool a) { if (a) return; else _ } + modifier mod1 { var a = 1; var b = 2; _; } + modifier mod2(bool a) { if (a) return; else _; } function f(bool a) mod1 mod2(a) returns (uint r) { return 3; } } )"; @@ -2379,7 +2379,7 @@ BOOST_AUTO_TEST_CASE(function_modifier_loop) { char const* sourceCode = R"( contract C { - modifier repeat(uint count) { for (var i = 0; i < count; ++i) _ } + modifier repeat(uint count) { for (var i = 0; i < count; ++i) _; } function f() repeat(10) returns (uint r) { r += 1; } } )"; @@ -2391,7 +2391,7 @@ BOOST_AUTO_TEST_CASE(function_modifier_multi_invocation) { char const* sourceCode = R"( contract C { - modifier repeat(bool twice) { if (twice) _ _ } + modifier repeat(bool twice) { if (twice) _; _; } function f(bool twice) repeat(twice) returns (uint r) { r += 1; } } )"; @@ -2406,7 +2406,7 @@ BOOST_AUTO_TEST_CASE(function_modifier_multi_with_return) // modifier code block. char const* sourceCode = R"( contract C { - modifier repeat(bool twice) { if (twice) _ _ } + modifier repeat(bool twice) { if (twice) _; _; } function f(bool twice) repeat(twice) returns (uint r) { r += 1; return r; } } )"; @@ -2420,10 +2420,10 @@ BOOST_AUTO_TEST_CASE(function_modifier_overriding) char const* sourceCode = R"( contract A { function f() mod returns (bool r) { return true; } - modifier mod { _ } + modifier mod { _; } } contract C is A { - modifier mod { if (false) _ } + modifier mod { if (false) _; } } )"; compileAndRun(sourceCode); @@ -2439,12 +2439,12 @@ BOOST_AUTO_TEST_CASE(function_modifier_calling_functions_in_creation_context) function f1() mod2 { data |= 0x1; } function f2() { data |= 0x20; } function f3() { } - modifier mod1 { f2(); _ } - modifier mod2 { f3(); if (false) _ } + modifier mod1 { f2(); _; } + modifier mod2 { f3(); if (false) _; } function getData() returns (uint r) { return data; } } contract C is A { - modifier mod1 { f4(); _ } + modifier mod1 { f4(); _; } function f3() { data |= 0x300; } function f4() { data |= 0x4000; } } @@ -2459,11 +2459,11 @@ BOOST_AUTO_TEST_CASE(function_modifier_for_constructor) contract A { uint data; function A() mod1 { data |= 2; } - modifier mod1 { data |= 1; _ } + modifier mod1 { data |= 1; _; } function getData() returns (uint r) { return data; } } contract C is A { - modifier mod1 { data |= 4; _ } + modifier mod1 { data |= 4; _; } } )"; compileAndRun(sourceCode); @@ -6920,7 +6920,7 @@ BOOST_AUTO_TEST_CASE(return_does_not_skip_modifier) contract C { uint public x; modifier setsx { - _ + _; x = 9; } function f() setsx returns (uint) { @@ -6941,7 +6941,7 @@ BOOST_AUTO_TEST_CASE(break_in_modifier) uint public x; modifier run() { for (uint i = 0; i < 10; i++) { - _ + _; break; } } @@ -6963,7 +6963,7 @@ BOOST_AUTO_TEST_CASE(stacked_return_with_modifiers) uint public x; modifier run() { for (uint i = 0; i < 10; i++) { - _ + _; break; } } @@ -6986,7 +6986,7 @@ BOOST_AUTO_TEST_CASE(mutex) modifier protected { if (locked) throw; locked = true; - _ + _; locked = false; } } diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 2c126b65..8d4890d1 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -859,8 +859,8 @@ BOOST_AUTO_TEST_CASE(function_modifier_invocation) char const* text = R"( contract B { function f() mod1(2, true) mod2("0123456") { } - modifier mod1(uint a, bool b) { if (b) _ } - modifier mod2(bytes7 a) { while (a == "1234567") _ } + modifier mod1(uint a, bool b) { if (b) _; } + modifier mod2(bytes7 a) { while (a == "1234567") _; } } )"; BOOST_CHECK(success(text)); @@ -871,7 +871,7 @@ BOOST_AUTO_TEST_CASE(invalid_function_modifier_type) char const* text = R"( contract B { function f() mod1(true) { } - modifier mod1(uint a) { if (a > 0) _ } + modifier mod1(uint a) { if (a > 0) _; } } )"; BOOST_CHECK(expectError(text) == Error::Type::TypeError); @@ -882,8 +882,8 @@ BOOST_AUTO_TEST_CASE(function_modifier_invocation_parameters) char const* text = R"( contract B { function f(uint8 a) mod1(a, true) mod2(r) returns (bytes7 r) { } - modifier mod1(uint a, bool b) { if (b) _ } - modifier mod2(bytes7 a) { while (a == "1234567") _ } + modifier mod1(uint a, bool b) { if (b) _; } + modifier mod2(bytes7 a) { while (a == "1234567") _; } } )"; BOOST_CHECK(success(text)); @@ -894,7 +894,7 @@ BOOST_AUTO_TEST_CASE(function_modifier_invocation_local_variables) char const* text = R"( contract B { function f() mod(x) { uint x = 7; } - modifier mod(uint a) { if (a > 0) _ } + modifier mod(uint a) { if (a > 0) _; } } )"; BOOST_CHECK(success(text)); @@ -903,8 +903,8 @@ BOOST_AUTO_TEST_CASE(function_modifier_invocation_local_variables) BOOST_AUTO_TEST_CASE(legal_modifier_override) { char const* text = R"( - contract A { modifier mod(uint a) { _ } } - contract B is A { modifier mod(uint a) { _ } } + contract A { modifier mod(uint a) { _; } } + contract B is A { modifier mod(uint a) { _; } } )"; BOOST_CHECK(success(text)); } @@ -912,8 +912,8 @@ BOOST_AUTO_TEST_CASE(legal_modifier_override) BOOST_AUTO_TEST_CASE(illegal_modifier_override) { char const* text = R"( - contract A { modifier mod(uint a) { _ } } - contract B is A { modifier mod(uint8 a) { _ } } + contract A { modifier mod(uint a) { _; } } + contract B is A { modifier mod(uint8 a) { _; } } )"; BOOST_CHECK(expectError(text) == Error::Type::TypeError); } @@ -921,7 +921,7 @@ BOOST_AUTO_TEST_CASE(illegal_modifier_override) BOOST_AUTO_TEST_CASE(modifier_overrides_function) { char const* text = R"( - contract A { modifier mod(uint a) { _ } } + contract A { modifier mod(uint a) { _; } } contract B is A { function mod(uint a) { } } )"; BOOST_CHECK(expectError(text) == Error::Type::TypeError); @@ -931,7 +931,7 @@ BOOST_AUTO_TEST_CASE(function_overrides_modifier) { char const* text = R"( contract A { function mod(uint a) { } } - contract B is A { modifier mod(uint a) { _ } } + contract B is A { modifier mod(uint a) { _; } } )"; BOOST_CHECK(expectError(text) == Error::Type::TypeError); } @@ -941,7 +941,7 @@ BOOST_AUTO_TEST_CASE(modifier_returns_value) char const* text = R"( contract A { function f(uint a) mod(2) returns (uint r) { } - modifier mod(uint a) { _ return 7; } + modifier mod(uint a) { _; return 7; } } )"; BOOST_CHECK(expectError(text) == Error::Type::TypeError); diff --git a/test/libsolidity/SolidityParser.cpp b/test/libsolidity/SolidityParser.cpp index 92f5a142..0c0e343b 100644 --- a/test/libsolidity/SolidityParser.cpp +++ b/test/libsolidity/SolidityParser.cpp @@ -681,15 +681,23 @@ BOOST_AUTO_TEST_CASE(placeholder_in_function_context) BOOST_AUTO_TEST_CASE(modifier) { char const* text = "contract c {\n" - " modifier mod { if (msg.sender == 0) _ }\n" + " modifier mod { if (msg.sender == 0) _; }\n" "}\n"; BOOST_CHECK(successParse(text)); } +BOOST_AUTO_TEST_CASE(modifier_without_semicolon) +{ + char const* text = "contract c {\n" + " modifier mod { if (msg.sender == 0) _ }\n" + "}\n"; + BOOST_CHECK(!successParse(text)); +} + BOOST_AUTO_TEST_CASE(modifier_arguments) { char const* text = "contract c {\n" - " modifier mod(uint a) { if (msg.sender == a) _ }\n" + " modifier mod(uint a) { if (msg.sender == a) _; }\n" "}\n"; BOOST_CHECK(successParse(text)); } @@ -697,8 +705,8 @@ BOOST_AUTO_TEST_CASE(modifier_arguments) BOOST_AUTO_TEST_CASE(modifier_invocation) { char const* text = "contract c {\n" - " modifier mod1(uint a) { if (msg.sender == a) _ }\n" - " modifier mod2 { if (msg.sender == 2) _ }\n" + " modifier mod1(uint a) { if (msg.sender == a) _; }\n" + " modifier mod2 { if (msg.sender == 2) _; }\n" " function f() mod1(7) mod2 { }\n" "}\n"; BOOST_CHECK(successParse(text)); -- cgit From 3b2174f7a82fec7ed8ef2e55ec00996fac32c948 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 5 Sep 2016 16:13:31 +0200 Subject: Update grammar.txt to reflect the change. --- libsolidity/grammar.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libsolidity/grammar.txt b/libsolidity/grammar.txt index 0230729a..86df3db0 100644 --- a/libsolidity/grammar.txt +++ b/libsolidity/grammar.txt @@ -41,8 +41,9 @@ ArrayTypeName = TypeName StorageLocation? '[' Expression? ']' StorageLocation = 'memory' | 'storage' Block = '{' Statement* '}' -Statement = IfStatement | WhileStatement | ForStatement | Block | PlaceholderStatement | - ( Continue | Break | Return | Throw | SimpleStatement | ExpressionStatement ) ';' +Statement = IfStatement | WhileStatement | ForStatement | Block | + ( PlaceholderStatement | Continue | Break | Return | + Throw | SimpleStatement | ExpressionStatement ) ';' ExpressionStatement = Expression | VariableDefinition IfStatement = 'if' '(' Expression ')' Statement ( 'else' Statement )? -- cgit