Handle syntax errors in parser

integration/program.cpp

@@ -110,7 +110,7 @@ class RunPythonProgram : public ::testing::Test
 
 		auto lexer = Lexer::create(std::string(program), "_integration_dummy_.py");
 		parser::Parser p{ lexer };
-		p.parse();
+		ASSERT_TRUE(p.parse().is_ok());
 		p.module()->print_node("");
 		m_bytecode = compiler::compile(p.module(),
 			{},

integration/run_python_tests.sh

@@ -46,4 +46,38 @@ else
     echo $file "... PASSED!"
 fi
 
+# A syntax error must exit non-zero and report the line the parser actually gave
+# up on -- not line 1 -- with a caret under the offending token.
+file=$SCRIPT_DIR/tests/expected_failures/syntax_error_reporting.py
+output=$(timeout 10s $PYTHON_EXECUTABLE $file --gc-frequency $GC_FREQUENCY 2>&1)
+if [ $? -eq 0 ]; then
+    echo $file "... FAILED! (expected a non-zero exit code)"
+    exit_code=1
+elif ! echo "$output" | grep -q '", line 4$'; then
+    echo $file "... FAILED! (expected the error on line 4, got: ${output})"
+    exit_code=1
+elif ! echo "$output" | grep -qF '            ^'; then
+    echo $file "... FAILED! (expected a caret under the ':', got: ${output})"
+    exit_code=1
+elif ! echo "$output" | grep -q '^SyntaxError: invalid syntax$'; then
+    echo $file "... FAILED! (expected a SyntaxError, got: ${output})"
+    exit_code=1
+else
+    echo $file "... PASSED!"
+fi
+
+# Running off the end of the input puts the caret just past the last character.
+file=$SCRIPT_DIR/tests/expected_failures/syntax_error_end_of_line.py
+output=$(timeout 10s $PYTHON_EXECUTABLE $file --gc-frequency $GC_FREQUENCY 2>&1)
+if [ $? -eq 0 ]; then
+    echo $file "... FAILED! (expected a non-zero exit code)"
+    exit_code=1
+elif ! echo "$output" | grep -qxF '          ^'; then
+    echo $file "... FAILED! (expected a caret after the ',', got: ${output})"
+    exit_code=1
+else
+    echo $file "... PASSED!"
+fi
+
+
 exit $exit_code

integration/tests/expected_failures/syntax_error_end_of_line.py

@@ -0,0 +1,2 @@
+x = 1
+foo(1,

integration/tests/expected_failures/syntax_error_reporting.py

@@ -0,0 +1,5 @@
+x = 1
+y = 2
+
+def foo(:
+    pass

integration/tests/syntax_error_attributes.py

@@ -0,0 +1,60 @@
+# SyntaxError carries CPython's (msg, filename, lineno, offset, text) attribute
+# set, and the two-argument form is what both Python code and the parser use.
+e = SyntaxError("invalid syntax", ("bad.py", 5, 9, "def foo(:"))
+assert e.args == ("invalid syntax", ("bad.py", 5, 9, "def foo(:")), e.args
+assert e.msg == "invalid syntax", e.msg
+assert e.filename == "bad.py", e.filename
+assert e.lineno == 5, e.lineno
+assert e.offset == 9, e.offset
+assert e.text == "def foo(:", e.text
+assert str(e) == "invalid syntax (bad.py, line 5)", str(e)
+
+# Without an info tuple the location attributes stay None and str() is the message.
+bare = SyntaxError("boom")
+assert bare.args == ("boom",), bare.args
+assert bare.msg == "boom", bare.msg
+assert bare.filename is None, bare.filename
+assert bare.lineno is None, bare.lineno
+assert bare.offset is None, bare.offset
+assert bare.text is None, bare.text
+assert str(bare) == "boom", str(bare)
+
+# With no arguments at all every attribute, msg included, is None.
+empty = SyntaxError()
+assert empty.msg is None, empty.msg
+assert empty.filename is None, empty.filename
+assert empty.lineno is None, empty.lineno
+assert empty.offset is None, empty.offset
+assert empty.text is None, empty.text
+
+# Python 3.9 reports a short info tuple as an IndexError.
+try:
+    SyntaxError("x", ("f.py", 1))
+    assert False, "expected IndexError"
+except IndexError as error:
+    assert str(error) == "tuple index out of range", str(error)
+
+try:
+    SyntaxError("x", foo=1)
+    assert False, "expected TypeError"
+except TypeError as error:
+    assert str(error) == "SyntaxError() takes no keyword arguments", str(error)
+
+# The attributes are writable, so nothing may assume their type.
+e.lineno = "not an int"
+assert str(e) == "invalid syntax (bad.py)", str(e)
+
+# compile() raises the parser's SyntaxError instead of aborting.
+try:
+    compile("def foo(:", "<string>", "exec")
+    assert False, "expected SyntaxError"
+except SyntaxError as error:
+    assert error.msg == "invalid syntax", error.msg
+    assert error.lineno == 1, error.lineno
+    assert error.offset == 9, error.offset
+
+# lineno is user-supplied, so out-of-range values are printed, not asserted on.
+negative = SyntaxError("m", ("f.py", -1, 1, "code"))
+assert str(negative) == "m (f.py, line -1)", str(negative)
+huge = SyntaxError("m", ("f.py", 10**30, 1, "code"))
+assert str(huge) == "m (f.py, line 1000000000000000000000000000000)", str(huge)

src/ast/optimizers/Optimizers_tests.cpp

@@ -466,7 +466,7 @@ void assert_generates_ast(std::string_view program,
 	parser::Parser p{ lexer };
 	const auto spdlog_level = spdlog::get_level();
 	spdlog::set_level(spdlog::level::debug);
-	p.parse();
+	ASSERT(p.parse().is_ok());
 	spdlog::set_level(spdlog_level);
 
 	if (lvl > compiler::OptimizationLevel::None) {

src/executable/bytecode/BytecodeProgram_tests.cpp

@@ -14,7 +14,7 @@ std::shared_ptr<BytecodeProgram> generate_bytecode(std::string_view program)
 {
 	auto lexer = Lexer::create(std::string(program), "_bytecode_program_tests_.py");
 	parser::Parser p{ lexer };
-	p.parse();
+	ASSERT(p.parse().is_ok());
 
 	auto module = p.module();
 	ASSERT(module);

src/executable/bytecode/codegen/BytecodeGenerator_tests.cpp

@@ -15,7 +15,7 @@ std::shared_ptr<BytecodeProgram> generate_bytecode(std::string_view program)
 {
 	auto lexer = Lexer::create(std::string(program), "_bytecode_generator_tests_.py");
 	parser::Parser p{ lexer };
-	p.parse();
+	ASSERT(p.parse().is_ok());
 
 	auto module = p.module();
 	ASSERT(module);

src/executable/bytecode/codegen/VariablesResolver_tests.cpp

@@ -13,7 +13,7 @@ VariablesResolver::VisibilityMap generate_resolver(std::string_view program)
 {
 	auto lexer = Lexer::create(std::string(program), "_bytecode_generator_tests_.py");
 	parser::Parser p{ lexer };
-	p.parse();
+	ASSERT(p.parse().is_ok());
 
 	auto *module = as<ast::Module>(p.module().get());
 	ASSERT(module);

src/executable/llvm/LLVMGenerator_tests.cpp

@@ -14,7 +14,7 @@ std::shared_ptr<Program> generate_llvm_module(std::string_view program)
 {
 	auto lexer = Lexer::create(std::string(program), "_llvm_backend_tests_.py");
 	parser::Parser p{ lexer };
-	p.parse();
+	ASSERT(p.parse().is_ok());
 
 	auto module = as<ast::Module>(p.module());
 	ASSERT(module);

src/parser/Parser.cpp

@@ -1,7 +1,8 @@
 module;
 #include "core.hpp"
-#include "spdlog/spdlog.h"
+#include "runtime/SourceManager.hpp"
 
+#include "spdlog/spdlog.h"
 #include <gmpxx.h>
 
 
@@ -133,8 +134,8 @@ template<typename Derived> struct PatternV2
 		if constexpr (seeds_sentinel) {
 			if (result.has_value()) {
 				// Safe to hold across grow_lr: memo entries live in a deque.
-				auto *slot = p.memo_find(start_position, memo_rule_id<Derived>);
-				ASSERT(slot);
+				const auto &slot = p.memo_find(start_position, memo_rule_id<Derived>);
+				ASSERT(slot.has_value());
 				ASSERT(slot->has_value());
 				auto &value = *slot;
 				if (std::holds_alternative<bool>(value->value) && std::get<bool>(value->value)) {
@@ -212,7 +213,8 @@ template<size_t TypeIdx, typename PatternTuple, typename = void> class PatternMa
 		if (!t.has_value()) { return {}; }
 		if constexpr (::detail::has_type<typename ResultTypeHead::value_type,
 						  ::detail::ValueTypesTuple>{}) {
-			if (auto *slot = p.memo_find(original_token_position, memo_rule_id<CurrentType>)) {
+			if (const auto &slot = p.memo_find(original_token_position, memo_rule_id<CurrentType>);
+				slot.has_value()) {
 				if (!slot->has_value()) { return {}; }
 				auto &value = (*slot)->value;
 				p.token_position() = (*slot)->position;
@@ -708,6 +710,7 @@ struct SingleTokenPatternV2 : PatternV2<SingleTokenPatternV2<Patterns...>>
 
 	static std::optional<ResultType> matches_impl(Parser &p)
 	{
+		p.observe_token(p.token_position());
 		if (SingleTokenPattern_<ComposedTypes<Patterns...>>::match(p)) {
 			const auto &t = p.lexer().peek_token(p.token_position());
 			return t.has_value() ? std::make_optional(ResultType{ *t }) : std::nullopt;
@@ -7389,33 +7392,41 @@ struct FilePattern : PatternV2<FilePattern>
 			}
 			return p.module();
 		}
-		size_t idx = 0;
-		auto t = *p.lexer().peek_token(idx);
-		auto begin = t.start().pointer_to_program;
-		auto end = t.end().pointer_to_program;
-		const size_t row = t.start().row;
-		while (row == t.start().row) {
-			end = t.end().pointer_to_program;
-			idx++;
-			t = *p.lexer().peek_token(idx);
-		}
-		std::string line{ begin, end };
-		spdlog::error("Syntax error on line {}: '{}'", row + 1, line);
-		// PARSER_ERROR();
 		return {};
 	}
 };
 
 namespace parser {
-void Parser::parse()
+PyResult<std::shared_ptr<ast::Module>> Parser::parse()
 {
 	auto result = PatternMatchV2<FilePattern>::match(*this);
 	if (result) {
 		auto [module] = *result;
 		m_module = std::move(module);
-		m_module->print_node("");
+		return Ok(m_module);
 	}
-	DEBUG_LOG("Parser return code: {}", result.has_value());
+	std::size_t index = m_furthest_token;
+	std::optional<Token> token = m_lexer.peek_token(index);
+	while (!token.has_value() && index > 0) { token = m_lexer.peek_token(--index); }
+
+	const auto &filename = m_lexer.filename();
+	const auto &program = m_lexer.program();
+	auto lineno = token.has_value() ? token->start().row + 1 : 1;
+	auto offset = token.has_value() ? token->start().column + 1 : 1;
+	const auto line_count =
+		std::max(static_cast<std::size_t>(std::count(program.begin(), program.end(), '\n'))
+					 + (program.empty() || program.back() == '\n' ? 0uz : 1uz),
+			1uz);
+	if (lineno > line_count) {
+		lineno = line_count;
+		offset = SourceManager::the().line(filename, lineno).size() + 1;
+	}
+	const auto text = SourceManager::the().line(filename, lineno);
+	return Err(syntax_error("invalid syntax",
+		SyntaxErrorLocation{ .filename = filename,
+			.lineno = lineno,
+			.offset = offset,
+			.text = std::string{ text } }));
 }
 
 PyResult<std::shared_ptr<ast::Module>> Parser::parse_expression()

src/parser/Parser.cppm

@@ -12,6 +12,7 @@ class Parser
 	std::shared_ptr<ast::Module> m_module;
 	Lexer &m_lexer;
 	std::size_t m_token_position{ 0 };
+	std::size_t m_furthest_token{ m_token_position };
 
   public:
 	struct CacheValue
@@ -25,18 +26,20 @@ class Parser
 
 	using MemoSlot = std::optional<CacheValue>;
 
-	MemoSlot *memo_find(std::size_t position, std::uint16_t rule)
+	std::optional<MemoSlot &> memo_find(std::size_t position, std::uint16_t rule)
 	{
-		if (position >= m_memo_index.size()) { return nullptr; }
+		if (position >= m_memo_index.size()) { return std::nullopt; }
 		for (const auto &[id, slot] : m_memo_index[position]) {
-			if (id == rule) { return &m_memo_pool[slot]; }
+			if (id == rule) { return m_memo_pool[slot]; }
 		}
-		return nullptr;
+		return std::nullopt;
 	}
 
 	MemoSlot &memo_insert(std::size_t position, std::uint16_t rule)
 	{
-		if (auto *existing = memo_find(position, rule)) { return *existing; }
+		if (const auto &existing = memo_find(position, rule); existing.has_value()) {
+			return existing.value();
+		}
 		if (position >= m_memo_index.size()) { m_memo_index.resize(position + 1); }
 		m_memo_pool.emplace_back();
 		m_memo_index[position].emplace_back(
@@ -65,8 +68,15 @@ class Parser
 	const std::size_t &token_position() const { return m_token_position; }
 	std::size_t &token_position() { return m_token_position; }
 
+	std::size_t furthest_token() const { return m_furthest_token; }
+
+	void observe_token(std::size_t position)
+	{
+		m_furthest_token = std::max(m_furthest_token, position);
+	}
+
 	// parses a file
-	void parse();
+	py::PyResult<std::shared_ptr<ast::Module>> parse();
 
 	// parses an expression used by the builtin `eval` function
 	py::PyResult<std::shared_ptr<ast::Module>> parse_expression();

src/parser/Parser_tests.cpp

@@ -1104,7 +1104,7 @@ void assert_generates_ast(std::string_view program, std::shared_ptr<Module> expe
 {
 	auto lexer = Lexer::create(std::string(program), "_parser_test_.py");
 	parser::Parser p{ lexer };
-	p.parse();
+	ASSERT_TRUE(p.parse().is_ok());
 	ASSERT_TRUE(p.module());
 
 	const auto lvl = spdlog::get_level();

src/repl/repl.cpp

@@ -89,16 +89,20 @@ int run_and_execute_script(size_t argc,
 		std::cout << std::endl;
 	}
 	parser::Parser p{ lexer };
-	p.parse();
+	auto module_ = p.parse();
+	if (module_.is_err()) {
+		std::cout << module_.unwrap_err()->format_traceback() << std::endl;
+		return EXIT_FAILURE;
+	}
 	if (print_ast) {
 		const auto lvl = spdlog::get_level();
 		spdlog::set_level(spdlog::level::debug);
-		p.module()->print_node("");
+		module_.unwrap()->print_node("");
 		spdlog::set_level(lvl);
 	}
 
 	std::shared_ptr<Program> bytecode = compiler::compile(
-		p.module(), argv_vector, compiler::Backend::MLIR, compiler::OptimizationLevel::None);
+		module_.unwrap(), argv_vector, compiler::Backend::MLIR, compiler::OptimizationLevel::None);
 
 	if (print_bytecode) {
 		std::cout << "Generated bytecode: \n";
@@ -108,7 +112,7 @@ int run_and_execute_script(size_t argc,
 	if (use_llvm) {
 #ifdef USE_LLVM
 		auto llvm_code = codegen::LLVMGenerator::compile(
-			p.module(), argv_vector, compiler::OptimizationLevel::None);
+			module_.unwrap(), argv_vector, compiler::OptimizationLevel::None);
 		if (!llvm_code) {
 			std::cout << "Could not compile to LLVM IR\n";
 		} else {

src/runtime/BaseException.cpp

@@ -107,9 +107,9 @@ std::string BaseException::to_string() const
 std::string BaseException::format_traceback() const
 {
 	std::ostringstream out;
-	out << "Traceback (most recent call last):\n";
 	auto *tb = m_traceback;
-	while (tb) {
+	if (tb) { out << "Traceback (most recent call last):\n"; }
+	for (; tb; tb = tb->m_tb_next) {
 		const auto &filename = tb->m_tb_frame->code()->m_filename;
 		out << std::format("  File \"{}\", line {}, in {}\n",
 			filename,
@@ -118,12 +118,17 @@ std::string BaseException::format_traceback() const
 		const auto source = SourceManager::the().line(filename, tb->m_tb_lineno);
 		const auto trimmed = SourceManager::strip_leading_whitespace(source);
 		if (!trimmed.empty()) { out << "    " << trimmed << "\n"; }
-		tb = tb->m_tb_next;
 	}
-	out << type()->name() << ": " << what() << "\n";
+	out << format_exception_only();
 	return out.str();
 }
 
+std::string BaseException::format_exception_only() const
+{
+	return std::format("{}: {}\n", type()->name(), what());
+}
+
+
 PyResult<PyObject *> BaseException::__repr__() const
 {
 	std::string args_part;

src/runtime/BaseException.cppm

@@ -72,6 +72,8 @@ class BaseException : public PyBaseObject
 	PyType *static_type() const override;
 	static PyType *class_type();
 
+	virtual std::string format_exception_only() const;
+
 	void visit_graph(Visitor &) override;
 };