diff --git a/Changelog.md b/Changelog.md index 4fa6ae558..c31aced2a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -4,6 +4,7 @@ Breaking Changes: * Disallow conversions between bytesX and uintY of different size. * Commandline interface: Require ``-`` if standard input is used as source. + * General: ``continue`` in a ``do...while`` loop jumps to the condition (it used to jump to the loop body). Warning: this may silently change the semantics of existing code. * Type Checker: Disallow arithmetic operations for Boolean variables. Features: diff --git a/libsolidity/analysis/ControlFlowBuilder.cpp b/libsolidity/analysis/ControlFlowBuilder.cpp index 35d7687c2..5bd39da39 100644 --- a/libsolidity/analysis/ControlFlowBuilder.cpp +++ b/libsolidity/analysis/ControlFlowBuilder.cpp @@ -159,15 +159,14 @@ bool ControlFlowBuilder::visit(WhileStatement const& _whileStatement) { auto afterWhile = newLabel(); auto whileBody = createLabelHere(); + auto condition = newLabel(); { - // Note that "continue" in this case currently indeed jumps to whileBody - // and not to the condition. This is inconsistent with JavaScript and C and - // therefore a bug. This will be fixed in the future (planned for 0.5.0) - // and the Control Flow Graph will have to be adjusted accordingly. - BreakContinueScope scope(*this, afterWhile, whileBody); + BreakContinueScope scope(*this, afterWhile, condition); appendControlFlow(_whileStatement.body()); } + + placeAndConnectLabel(condition); appendControlFlow(_whileStatement.condition()); connect(m_currentNode, whileBody); diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 0889ac7ce..f195b4165 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -666,32 +666,36 @@ bool ContractCompiler::visit(WhileStatement const& _whileStatement) { StackHeightChecker checker(m_context); CompilerContext::LocationSetter locationSetter(m_context, _whileStatement); + eth::AssemblyItem loopStart = m_context.newTag(); eth::AssemblyItem loopEnd = m_context.newTag(); - m_continueTags.push_back(loopStart); m_breakTags.push_back(loopEnd); m_context << loopStart; - // While loops have the condition prepended - if (!_whileStatement.isDoWhile()) - { - compileExpression(_whileStatement.condition()); - m_context << Instruction::ISZERO; - m_context.appendConditionalJumpTo(loopEnd); - } - - _whileStatement.body().accept(*this); - - // Do-while loops have the condition appended if (_whileStatement.isDoWhile()) { + eth::AssemblyItem condition = m_context.newTag(); + m_continueTags.push_back(condition); + + _whileStatement.body().accept(*this); + + m_context << condition; + compileExpression(_whileStatement.condition()); + m_context << Instruction::ISZERO << Instruction::ISZERO; + m_context.appendConditionalJumpTo(loopStart); + } + else + { + m_continueTags.push_back(loopStart); compileExpression(_whileStatement.condition()); m_context << Instruction::ISZERO; m_context.appendConditionalJumpTo(loopEnd); - } - m_context.appendJumpTo(loopStart); + _whileStatement.body().accept(*this); + + m_context.appendJumpTo(loopStart); + } m_context << loopEnd; m_continueTags.pop_back(); diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 368405683..88a1756b7 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -482,6 +482,27 @@ BOOST_AUTO_TEST_CASE(do_while_loop) testContractAgainstCppOnRange("f(uint256)", do_while_loop_cpp, 0, 5); } +BOOST_AUTO_TEST_CASE(do_while_loop_continue) +{ + char const* sourceCode = R"( + contract test { + function f() public pure returns(uint r) { + uint i = 0; + do + { + if (i > 0) return 0; + i++; + continue; + } while (false); + return 42; + } + } + )"; + compileAndRun(sourceCode); + + ABI_CHECK(callContractFunction("f()"), encodeArgs(42)); +} + BOOST_AUTO_TEST_CASE(nested_loops) { // tests that break and continue statements in nested loops jump to the correct place diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_fine.sol index 6520672cf..55c5edd30 100644 --- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_fine.sol +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_fine.sol @@ -23,13 +23,8 @@ contract C { } function k() internal view returns (S storage c) { do { - if (s.f) { - continue; - break; - } - else { - c = s; - } + c = s; + continue; } while(false); } } diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_warn.sol index f1a92e9c2..7d001c194 100644 --- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_warn.sol +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/dowhile_warn.sol @@ -21,6 +21,15 @@ contract C { do { if (s.f) { break; + } + else { + c = s; + } + } while(false); + } + function i() internal view returns (S storage c) { + do { + if (s.f) { continue; } else { @@ -28,8 +37,16 @@ contract C { } } while(false); } + function j() internal view returns (S storage c) { + do { + continue; + c = s; + } while(false); + } } // ---- // Warning: (87-98): 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. // Warning: (223-234): 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. // Warning: (440-451): 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. +// Warning: (654-665): 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. +// Warning: (871-882): 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.