Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| parser::Parser p{ lexer }; | ||
| p.parse(); | ||
| auto module_ = p.parse(); | ||
| if (module_.is_err()) { TODO(); } |
There was a problem hiding this comment.
The compile() builtin aborts the whole process instead of raising a SyntaxError (bug)
module_.is_err() calls TODO(), which aborts via std::abort(). Since p.parse() now actually returns a SyntaxError on failure, compile("def foo(:", "<string>", "exec") will crash the interpreter instead of raising a catchable SyntaxError, unlike every other caller in this PR (repl.cpp, freeze.cpp) which was updated to propagate the error. The enclosing function returns PyResult<PyObject *>, so this should be return Err(module_.unwrap_err());.
| const auto column = std::min(line.size(), offset->as_size_t()); | ||
| auto prefix = line.substr(0, column > 0 ? column - 1 : 0); |
There was a problem hiding this comment.
Caret rendered one column too far left when the error is at end-of-line (bug)
offset is 1-based over 1 .. line.size() + 1 (an offset of line.size() + 1 means "past the last character", which Parser::parse() deliberately produces for the ran-off-end-of-input case). Clamping with std::min(line.size(), offset->as_size_t()) caps column at line.size(), so when offset == line.size() + 1 the computed column is one short, and prefix/the caret end up one column left of the intended position. The clamp should be std::min(line.size() + 1, offset->as_size_t()).
| } | ||
| if (filename && lineno) { | ||
| return PyString::create( | ||
| std::format("{} ({}, line {})", msg, basename, lineno->as_size_t())); |
There was a problem hiding this comment.
Unvalidated user-supplied lineno/offset abort the process instead of raising (bug)
m_lineno/m_offset are set directly from the user-supplied info tuple in __init__ with no range/sign checks, and are also writable via the plain .attr("lineno", ...)/.attr("offset", ...) descriptors. __str__ and format_exception_only both call ->as_size_t() on them unconditionally, and PyInteger::as_size_t() asserts fits_ulong_p() and calls std::abort() otherwise. So e.g. str(SyntaxError("m", ("f.py", -1, 1, "code"))), or an uncaught raise SyntaxError("m", ("f.py", -1, 1, "code")), aborts the interpreter (SIGABRT) instead of behaving like CPython, which just prints the value.
Code reviewFound 3 new issues — see the inline comments. |
1 participant
No description provided.