check whether break/continue is in the loop

8 files changed, 187 insertions, 12 deletions

diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp
new file mode 100644
index 00000000..92743461
--- /dev/null
+++ b/libsolidity/analysis/SyntaxChecker.cpp
@@ -0,0 +1,87 @@
+/*
+	This file is part of cpp-ethereum.
+
+	cpp-ethereum is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 3 of the License, or
+	(at your option) any later version.
+
+	cpp-ethereum is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with cpp-ethereum.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <memory>
+#include <libsolidity/ast/AST.h>
+#include <libsolidity/analysis/SyntaxChecker.h>
+
+using namespace std;
+using namespace dev;
+using namespace dev::solidity;
+
+
+bool SyntaxChecker::checkSyntax(SourceUnit const& _sourceUnit)
+{
+	_sourceUnit.accept(*this);
+	return m_errors.empty();
+}
+
+void SyntaxChecker::syntaxError(SourceLocation const& _location, std::string const& _description)
+{
+	auto err = make_shared<Error>(Error::Type::SyntaxError);
+	*err <<
+		errinfo_sourceLocation(_location) <<
+		errinfo_comment(_description);
+
+	m_errors.push_back(err);
+}
+
+bool SyntaxChecker::visit(WhileStatement const& _whileStatement)
+{
+	_whileStatement.body().annotation().isInLoop = true;
+	return true;
+}
+
+bool SyntaxChecker::visit(ForStatement const& _forStatement)
+{
+	_forStatement.body().annotation().isInLoop = true;
+	return true;
+}
+
+bool SyntaxChecker::visit(Block const& _blockStatement)
+{
+	bool inLoop = _blockStatement.annotation().isInLoop;
+	for (auto& statement : _blockStatement.statements())
+		statement->annotation().isInLoop = inLoop;
+	return true;
+}
+
+bool SyntaxChecker::visit(IfStatement const& _ifStatement)
+{
+	bool inLoop = _ifStatement.annotation().isInLoop;
+	_ifStatement.trueStatement().annotation().isInLoop = inLoop;
+	if (_ifStatement.falseStatement())
+		_ifStatement.falseStatement()->annotation().isInLoop = inLoop;
+	return true;
+}
+
+bool SyntaxChecker::visit(Continue const& _continueStatement)
+{
+	if (!_continueStatement.annotation().isInLoop)
+		// we're not in a for/while loop, report syntax error
+		syntaxError(_continueStatement.location(), "\"continue\" has to be in a \"for\" or \"while\" loop.");
+	return true;
+}
+
+bool SyntaxChecker::visit(Break const& _breakStatement)
+{
+	if (!_breakStatement.annotation().isInLoop)
+		// we're not in a for/while loop, report syntax error
+		syntaxError(_breakStatement.location(), "\"break\" has to be in a \"for\" or \"while\" loop.");
+	return true;
+}
+
diff --git a/libsolidity/analysis/SyntaxChecker.h b/libsolidity/analysis/SyntaxChecker.h
new file mode 100644
index 00000000..66f6d94d
--- /dev/null
+++ b/libsolidity/analysis/SyntaxChecker.h
@@ -0,0 +1,58 @@
+/*
+	This file is part of cpp-ethereum.
+
+	cpp-ethereum is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 3 of the License, or
+	(at your option) any later version.
+
+	cpp-ethereum is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with cpp-ethereum.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#pragma once
+
+#include <libsolidity/analysis/TypeChecker.h>
+#include <libsolidity/ast/Types.h>
+#include <libsolidity/ast/ASTAnnotations.h>
+#include <libsolidity/ast/ASTForward.h>
+#include <libsolidity/ast/ASTVisitor.h>
+
+namespace dev
+{
+namespace solidity
+{
+
+/**
+ * The module that performs syntax analysis on the AST:
+ *  - whether continue/break is in a for/while loop.
+ */
+class SyntaxChecker : private ASTConstVisitor
+{
+public:
+	/// @param _errors the reference to the list of errors and warnings to add them found during type checking.
+	SyntaxChecker(ErrorList& _errors): m_errors(_errors) {}
+
+	bool checkSyntax(SourceUnit const& _sourceUnit);
+
+private:
+	/// Adds a new error to the list of errors.
+	void syntaxError(SourceLocation const& _location, std::string const& _description);
+
+	virtual bool visit(WhileStatement const& _whileStatement) override;
+	virtual bool visit(ForStatement const& _forStatement) override;
+	virtual bool visit(Block const& _blockStatement) override;
+	virtual bool visit(IfStatement const& _ifStatement) override;
+	virtual bool visit(Continue const& _continueStatement) override;
+	virtual bool visit(Break const& _breakStatement) override;
+
+	ErrorList& m_errors;
+};
+
+}
+}
diff --git a/libsolidity/ast/ASTAnnotations.h b/libsolidity/ast/ASTAnnotations.h
index 235338bb..cae606ca 100644
--- a/libsolidity/ast/ASTAnnotations.h
+++ b/libsolidity/ast/ASTAnnotations.h
@@ -108,6 +108,7 @@ struct VariableDeclarationAnnotation: ASTAnnotation
 
 struct StatementAnnotation: ASTAnnotation, DocumentedAnnotation
 {
+	bool isInLoop = false;
 };
 
 struct ReturnAnnotation: StatementAnnotation
diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp
index a6f6f224..8a0d2f5e 100644
--- a/libsolidity/interface/CompilerStack.cpp
+++ b/libsolidity/interface/CompilerStack.cpp
@@ -30,6 +30,7 @@
 #include <libsolidity/analysis/NameAndTypeResolver.h>
 #include <libsolidity/analysis/TypeChecker.h>
 #include <libsolidity/analysis/DocStringAnalyser.h>
+#include <libsolidity/analysis/SyntaxChecker.h>
 #include <libsolidity/codegen/Compiler.h>
 #include <libsolidity/interface/CompilerStack.h>
 #include <libsolidity/interface/InterfaceHandler.h>
@@ -122,6 +123,11 @@ bool CompilerStack::parse()
 	resolveImports();
 
 	bool noErrors = true;
+	SyntaxChecker syntaxChecker(m_errors);
+	for (Source const* source: m_sourceOrder)
+		if (!syntaxChecker.checkSyntax(*source->ast))
+			return false;
+
 	DocStringAnalyser docStringAnalyser(m_errors);
 	for (Source const* source: m_sourceOrder)
 		if (!docStringAnalyser.analyseDocStrings(*source->ast))
diff --git a/libsolidity/interface/Exceptions.cpp b/libsolidity/interface/Exceptions.cpp
index 465c3d2f..6d72520b 100644
--- a/libsolidity/interface/Exceptions.cpp
+++ b/libsolidity/interface/Exceptions.cpp
@@ -39,6 +39,9 @@ Error::Error(Type _type): m_type(_type)
 	case Type::ParserError:
 		m_typeName = "Parser Error";
 		break;
+	case Type::SyntaxError:
+		m_typeName = "Syntax Error";
+		break;
 	case Type::TypeError:
 		m_typeName = "Type Error";
 		break;
diff --git a/libsolidity/interface/Exceptions.h b/libsolidity/interface/Exceptions.h
index 14be3c3d..07835320 100644
--- a/libsolidity/interface/Exceptions.h
+++ b/libsolidity/interface/Exceptions.h
@@ -47,6 +47,7 @@ public:
 		DocstringParsingError,
 		ParserError,
 		TypeError,
+		SyntaxError,
 		Why3TranslatorError,
 		Warning
 	};
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 34c5dffc..3ef5ebbe 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -166,18 +166,6 @@ BOOST_AUTO_TEST_CASE(while_loop)
 	testSolidityAgainstCppOnRange("f(uint256)", while_loop_cpp, 0, 5);
 }
 
-BOOST_AUTO_TEST_CASE(break_outside_loop)
-{
-	// break and continue outside loops should be simply ignored
-	char const* sourceCode = "contract test {\n"
-							 "  function f(uint x) returns(uint y) {\n"
-							 "    break; continue; return 2;\n"
-							 "  }\n"
-							 "}\n";
-	compileAndRun(sourceCode);
-	testSolidityAgainstCpp("f(uint256)", [](u256 const&) -> u256 { return 2; }, u256(0));
-}
-
 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/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index 4697e756..11e78d94 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -27,6 +27,7 @@
 #include <libsolidity/parsing/Scanner.h>
 #include <libsolidity/parsing/Parser.h>
 #include <libsolidity/analysis/NameAndTypeResolver.h>
+#include <libsolidity/analysis/SyntaxChecker.h>
 #include <libsolidity/interface/Exceptions.h>
 #include <libsolidity/analysis/GlobalContext.h>
 #include <libsolidity/analysis/TypeChecker.h>
@@ -57,6 +58,10 @@ parseAnalyseAndReturnError(string const& _source, bool _reportWarnings = false)
 		if(!sourceUnit)
 			return make_pair(sourceUnit, nullptr);
 
+		SyntaxChecker syntaxChecker(errors);
+		if (!syntaxChecker.checkSyntax(*sourceUnit))
+			return make_pair(sourceUnit, std::make_shared<Error::Type const>(errors[0]->type()));
+
 		std::shared_ptr<GlobalContext> globalContext = make_shared<GlobalContext>();
 		NameAndTypeResolver resolver(globalContext->declarations(), errors);
 		solAssert(Error::containsOnlyWarnings(errors), "");
@@ -2903,6 +2908,32 @@ BOOST_AUTO_TEST_CASE(lvalues_as_inline_array)
 	BOOST_CHECK(expectError(text) == Error::Type::TypeError);
 }
 
+BOOST_AUTO_TEST_CASE(break_not_in_loop)
+{
+	char const* text = R"(
+		contract C {
+			function f() {
+				if (true)
+					break;
+			}
+		}
+	)";
+	BOOST_CHECK(expectError(text) == Error::Type::SyntaxError);
+}
+
+BOOST_AUTO_TEST_CASE(continue_not_in_loop)
+{
+	char const* text = R"(
+		contract C {
+			function f() {
+				if (true)
+					continue;
+			}
+		}
+	)";
+	BOOST_CHECK(expectError(text) == Error::Type::SyntaxError);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }