Merge pull request #6555 from sifmelcara/break-for-loop

[Yul] Disallow function definitions inside for loop init blocks

Changelog.md

@@ -7,6 +7,7 @@ Important Bugfixes:
 Language Features:
  * Code Generation: Implement copying recursive structs from storage to memory.
  * ABIEncoderV2: Implement encoding of calldata arrays and structs.
+ * Yul: Disallow function definitions inside for-loop init blocks.
 
 
 Compiler Features:

docs/yul.rst

@@ -167,6 +167,7 @@ The ``continue`` and ``break`` statements can only be used inside loop bodies
 and have to be in the same function as the loop (or both have to be at the
 top level).
 The condition part of the for-loop has to evaluate to exactly one value.
+Functions cannot be defined inside for loop init blocks.
 
 Literals cannot be larger than the their type. The largest type defined is 256-bit wide.
 

libyul/AsmParser.cpp

@@ -126,31 +126,19 @@ Statement Parser::parseStatement()
 	case Token::For:
 		return parseForLoop();
 	case Token::Break:
-		if (m_insideForLoopBody)
 	{
-			auto stmt = Statement{ createWithLocation<Break>() };
+		Statement stmt{createWithLocation<Break>()};
+		checkBreakContinuePosition("break");
 		m_scanner->next();
 		return stmt;
 	}
-		else
-		{
-			m_errorReporter.syntaxError(location(), "Keyword break outside for-loop body is not allowed.");
-			m_scanner->next();
-			return {};
-		}
 	case Token::Continue:
-		if (m_insideForLoopBody)
 	{
-			auto stmt = Statement{ createWithLocation<Continue>() };
+		Statement stmt{createWithLocation<Continue>()};
+		checkBreakContinuePosition("continue");
 		m_scanner->next();
 		return stmt;
 	}
-		else
-		{
-			m_errorReporter.syntaxError(location(), "Keyword continue outside for-loop body is not allowed.");
-			m_scanner->next();
-			return {};
-		}
 	case Token::Assign:
 	{
 		if (m_dialect->flavour != AsmFlavour::Loose)
@@ -290,20 +278,24 @@ Case Parser::parseCase()
 
 ForLoop Parser::parseForLoop()
 {
-	bool outerForLoopBody = m_insideForLoopBody;
-	m_insideForLoopBody = false;
-
 	RecursionGuard recursionGuard(*this);
+
+	ForLoopComponent outerForLoopComponent = m_currentForLoopComponent;
+
 	ForLoop forLoop = createWithLocation<ForLoop>();
 	expectToken(Token::For);
+	m_currentForLoopComponent = ForLoopComponent::ForLoopPre;
 	forLoop.pre = parseBlock();
+	m_currentForLoopComponent = ForLoopComponent::None;
 	forLoop.condition = make_unique<Expression>(parseExpression());
+	m_currentForLoopComponent = ForLoopComponent::ForLoopPost;
 	forLoop.post = parseBlock();
-
+	m_currentForLoopComponent = ForLoopComponent::ForLoopBody;
-	m_insideForLoopBody = true;
 	forLoop.body = parseBlock();
-	m_insideForLoopBody = outerForLoopBody;
 	forLoop.location.end = forLoop.body.location.end;
+
+	m_currentForLoopComponent = outerForLoopComponent;
+
 	return forLoop;
 }
 
@@ -487,9 +479,16 @@ VariableDeclaration Parser::parseVariableDeclaration()
 FunctionDefinition Parser::parseFunctionDefinition()
 {
 	RecursionGuard recursionGuard(*this);
-	auto outerForLoopBody = m_insideForLoopBody;
+
-	m_insideForLoopBody = false;
+	if (m_currentForLoopComponent == ForLoopComponent::ForLoopPre)
-	ScopeGuard restoreInsideForLoopBody{[&]() { m_insideForLoopBody = outerForLoopBody; }};
+		m_errorReporter.syntaxError(
+			location(),
+			"Functions cannot be defined inside a for-loop init block."
+		);
+
+	ForLoopComponent outerForLoopComponent = m_currentForLoopComponent;
+	m_currentForLoopComponent = ForLoopComponent::None;
+
 	FunctionDefinition funDef = createWithLocation<FunctionDefinition>();
 	expectToken(Token::Function);
 	funDef.name = expectAsmIdentifier();
@@ -516,6 +515,8 @@ FunctionDefinition Parser::parseFunctionDefinition()
 	}
 	funDef.body = parseBlock();
 	funDef.location.end = funDef.body.location.end;
+
+	m_currentForLoopComponent = outerForLoopComponent;
 	return funDef;
 }
 
@@ -642,6 +643,24 @@ YulString Parser::expectAsmIdentifier()
 	return name;
 }
 
+void Parser::checkBreakContinuePosition(string const& _which)
+{
+	switch (m_currentForLoopComponent)
+	{
+	case ForLoopComponent::None:
+		m_errorReporter.syntaxError(location(), "Keyword \"" + _which + "\" needs to be inside a for-loop body.");
+		break;
+	case ForLoopComponent::ForLoopPre:
+		m_errorReporter.syntaxError(location(), "Keyword \"" + _which + "\" in for-loop init block is not allowed.");
+		break;
+	case ForLoopComponent::ForLoopPost:
+		m_errorReporter.syntaxError(location(), "Keyword \"" + _which + "\" in for-loop post block is not allowed.");
+		break;
+	case ForLoopComponent::ForLoopBody:
+		break;
+	}
+}
+
 bool Parser::isValidNumberLiteral(string const& _literal)
 {
 	try

libyul/AsmParser.h

@@ -32,14 +32,20 @@
 #include <memory>
 #include <vector>
 
+
 namespace yul
 {
 
 class Parser: public langutil::ParserBase
 {
 public:
+	enum class ForLoopComponent
+	{
+		None, ForLoopPre, ForLoopPost, ForLoopBody
+	};
+
 	explicit Parser(langutil::ErrorReporter& _errorReporter, std::shared_ptr<Dialect> _dialect):
-		ParserBase(_errorReporter), m_dialect(std::move(_dialect)), m_insideForLoopBody{false} {}
+		ParserBase(_errorReporter), m_dialect(std::move(_dialect)) {}
 
 	/// Parses an inline assembly block starting with `{` and ending with `}`.
 	/// @param _reuseScanner if true, do check for end of input after the `}`.
@@ -85,11 +91,14 @@ protected:
 	TypedName parseTypedName();
 	YulString expectAsmIdentifier();
 
+	/// Reports an error if we are currently not inside the body part of a for loop.
+	void checkBreakContinuePosition(std::string const& _which);
+
 	static bool isValidNumberLiteral(std::string const& _literal);
 
 private:
 	std::shared_ptr<Dialect> m_dialect;
-	bool m_insideForLoopBody;
+	ForLoopComponent m_currentForLoopComponent = ForLoopComponent::None;
 };
 
 }

test/libyul/Inliner.cpp

@@ -82,8 +82,8 @@ BOOST_AUTO_TEST_CASE(simple_inside_structures)
 		"}"
 	"}"), "g,f");
 	BOOST_CHECK_EQUAL(inlinableFunctions("{"
-		"for {"
 		"function g(a:u256) -> b:u256 { b := a }"
+		"for {"
 		"} 1:u256 {"
 			"function f() -> x:u256 { x := g(2:u256) }"
 		"}"

test/libyul/Parser.cpp

@@ -295,6 +295,28 @@ BOOST_AUTO_TEST_CASE(if_statement)
 	BOOST_CHECK(successParse("{ function f() -> x:bool {} if f() { let b:bool := f() } }"));
 }
 
+BOOST_AUTO_TEST_CASE(break_outside_of_for_loop)
+{
+	auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion::constantinople());
+	CHECK_ERROR_DIALECT(
+		"{ let x if x { break } }",
+		SyntaxError,
+		"Keyword \"break\" needs to be inside a for-loop body.",
+		dialect
+	);
+}
+
+BOOST_AUTO_TEST_CASE(continue_outside_of_for_loop)
+{
+	auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion::constantinople());
+	CHECK_ERROR_DIALECT(
+		"{ let x if x { continue } }",
+		SyntaxError,
+		"Keyword \"continue\" needs to be inside a for-loop body.",
+		dialect
+	);
+}
+
 BOOST_AUTO_TEST_CASE(for_statement)
 {
 	auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion::constantinople());
@@ -313,8 +335,9 @@ BOOST_AUTO_TEST_CASE(for_statement_break_init)
 	CHECK_ERROR_DIALECT(
 		"{ for {let i := 0 break} iszero(eq(i, 10)) {i := add(i, 1)} {} }",
 		SyntaxError,
-		"Keyword break outside for-loop body is not allowed.",
+		"Keyword \"break\" in for-loop init block is not allowed.",
-		dialect);
+		dialect
+	);
 }
 
 BOOST_AUTO_TEST_CASE(for_statement_break_post)
@@ -323,8 +346,9 @@ BOOST_AUTO_TEST_CASE(for_statement_break_post)
 	CHECK_ERROR_DIALECT(
 		"{ for {let i := 0} iszero(eq(i, 10)) {i := add(i, 1) break} {} }",
 		SyntaxError,
-		"Keyword break outside for-loop body is not allowed.",
+		"Keyword \"break\" in for-loop post block is not allowed.",
-		dialect);
+		dialect
+	);
 }
 
 BOOST_AUTO_TEST_CASE(for_statement_nested_break)
@@ -333,8 +357,9 @@ BOOST_AUTO_TEST_CASE(for_statement_nested_break)
 	CHECK_ERROR_DIALECT(
 		"{ for {let i := 0} iszero(eq(i, 10)) {} { function f() { break } } }",
 		SyntaxError,
-		"Keyword break outside for-loop body is not allowed.",
+		"Keyword \"break\" needs to be inside a for-loop body.",
-		dialect);
+		dialect
+	);
 }
 
 BOOST_AUTO_TEST_CASE(for_statement_continue)
@@ -349,8 +374,9 @@ BOOST_AUTO_TEST_CASE(for_statement_continue_fail_init)
 	CHECK_ERROR_DIALECT(
 		"{ for {let i := 0 continue} iszero(eq(i, 10)) {i := add(i, 1)} {} }",
 		SyntaxError,
-		"Keyword continue outside for-loop body is not allowed.",
+		"Keyword \"continue\" in for-loop init block is not allowed.",
-		dialect);
+		dialect
+	);
 }
 
 BOOST_AUTO_TEST_CASE(for_statement_continue_fail_post)
@@ -359,8 +385,89 @@ BOOST_AUTO_TEST_CASE(for_statement_continue_fail_post)
 	CHECK_ERROR_DIALECT(
 		"{ for {let i := 0} iszero(eq(i, 10)) {i := add(i, 1) continue} {} }",
 		SyntaxError,
-		"Keyword continue outside for-loop body is not allowed.",
+		"Keyword \"continue\" in for-loop post block is not allowed.",
-		dialect);
+		dialect
+	);
+}
+
+BOOST_AUTO_TEST_CASE(for_statement_nested_continue)
+{
+	auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion::constantinople());
+	CHECK_ERROR_DIALECT(
+		"{ for {let i := 0} iszero(eq(i, 10)) {} { function f() { continue } } }",
+		SyntaxError,
+		"Keyword \"continue\" needs to be inside a for-loop body.",
+		dialect
+	);
+}
+
+BOOST_AUTO_TEST_CASE(for_statement_continue_nested_init_in_body)
+{
+	auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion::constantinople());
+	CHECK_ERROR_DIALECT(
+		"{ for {} 1 {} {let x for { continue } x {} {}} }",
+		SyntaxError,
+		"Keyword \"continue\" in for-loop init block is not allowed.",
+		dialect
+	);
+}
+
+BOOST_AUTO_TEST_CASE(for_statement_continue_nested_body_in_init)
+{
+	auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion{});
+	BOOST_CHECK(successParse("{ for {let x for {} x {} { continue }} 1 {} {} }", dialect));
+}
+
+BOOST_AUTO_TEST_CASE(for_statement_break_nested_body_in_init)
+{
+	auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion{});
+	BOOST_CHECK(successParse("{ for {let x for {} x {} { break }} 1 {} {} }", dialect));
+}
+
+BOOST_AUTO_TEST_CASE(for_statement_continue_nested_body_in_post)
+{
+	auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion{});
+	BOOST_CHECK(successParse("{ for {} 1 {let x for {} x {} { continue }} {} }", dialect));
+}
+
+BOOST_AUTO_TEST_CASE(for_statement_break_nested_body_in_post)
+{
+	auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion{});
+	BOOST_CHECK(successParse("{ for {} 1 {let x for {} x {} { break }} {} }", dialect));
+}
+
+BOOST_AUTO_TEST_CASE(function_defined_in_init_block)
+{
+	auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion{});
+	BOOST_CHECK(successParse("{ for { } 1 { function f() {} } {} }", dialect));
+	BOOST_CHECK(successParse("{ for { } 1 {} { function f() {} } }", dialect));
+	CHECK_ERROR_DIALECT(
+		"{ for { function f() {} } 1 {} {} }",
+		SyntaxError,
+		"Functions cannot be defined inside a for-loop init block.",
+		dialect
+	);
+}
+
+BOOST_AUTO_TEST_CASE(function_defined_in_init_nested)
+{
+	auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion{});
+	BOOST_CHECK(successParse(
+		"{ for {"
+			"for { } 1 { function f() {} } {}"
+		"} 1 {} {} }", dialect));
+	CHECK_ERROR_DIALECT(
+		"{ for { for {function foo() {}} 1 {} {} } 1 {} {} }",
+		SyntaxError,
+		"Functions cannot be defined inside a for-loop init block.",
+		dialect
+	);
+	CHECK_ERROR_DIALECT(
+		"{ for {} 1 {for {function foo() {}} 1 {} {} } {} }",
+		SyntaxError,
+		"Functions cannot be defined inside a for-loop init block.",
+		dialect
+	);
 }
 
 BOOST_AUTO_TEST_CASE(if_statement_invalid)