From b3deb0f304710a016a210a6aab9f4a935b5b60c2 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 11 Sep 2024 20:22:43 +0100 Subject: [PATCH 1/7] gh-123969: refactor _PyErr_RaiseSyntaxError and _PyErr_EmitSyntaxWarning out of compiler --- Include/internal/pycore_compile.h | 2 -- Include/internal/pycore_pyerrors.h | 5 +++ Python/compile.c | 39 +++++------------------- Python/errors.c | 49 ++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 33 deletions(-) diff --git a/Include/internal/pycore_compile.h b/Include/internal/pycore_compile.h index 9219d4d735a5764..d4c8c5dd850799d 100644 --- a/Include/internal/pycore_compile.h +++ b/Include/internal/pycore_compile.h @@ -31,8 +31,6 @@ extern int _PyCompile_AstOptimize( int optimize, struct _arena *arena); -struct _Py_SourceLocation; - extern int _PyAST_Optimize( struct _mod *, struct _arena *arena, diff --git a/Include/internal/pycore_pyerrors.h b/Include/internal/pycore_pyerrors.h index 9835e495d176e7e..db61aa181a7b06e 100644 --- a/Include/internal/pycore_pyerrors.h +++ b/Include/internal/pycore_pyerrors.h @@ -120,6 +120,11 @@ extern void _PyErr_SetNone(PyThreadState *tstate, PyObject *exception); extern PyObject* _PyErr_NoMemory(PyThreadState *tstate); +extern int _PyErr_EmitSyntaxWarning(PyObject *msg, PyObject *filename, int lineno, int col_offset, + int end_lineno, int end_col_offset); +extern int _PyErr_RaiseSyntaxError(PyObject *msg, PyObject *filename, int lineno, int col_offset, + int end_lineno, int end_col_offset); + PyAPI_FUNC(void) _PyErr_SetString( PyThreadState *tstate, PyObject *exception, diff --git a/Python/compile.c b/Python/compile.c index 72399c758d51e86..3415cc857c5376e 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -1112,27 +1112,15 @@ _PyCompile_Error(compiler *c, location loc, const char *format, ...) if (msg == NULL) { return ERROR; } - PyObject *loc_obj = PyErr_ProgramTextObject(c->c_filename, loc.lineno); - if (loc_obj == NULL) { - loc_obj = Py_None; - } - PyObject *args = Py_BuildValue("O(OiiOii)", msg, c->c_filename, - loc.lineno, loc.col_offset + 1, loc_obj, - loc.end_lineno, loc.end_col_offset + 1); + int ret = _PyErr_RaiseSyntaxError(msg, c->c_filename, loc.lineno, loc.col_offset, + loc.end_lineno, loc.end_col_offset); Py_DECREF(msg); - if (args == NULL) { - goto exit; - } - PyErr_SetObject(PyExc_SyntaxError, args); - exit: - Py_DECREF(loc_obj); - Py_XDECREF(args); - return ERROR; + return ret; } -/* Emits a SyntaxWarning and returns 1 on success. +/* Emits a SyntaxWarning and returns 0 on success. If a SyntaxWarning raised as error, replaces it with a SyntaxError - and returns 0. + and returns -1. */ int _PyCompile_Warn(compiler *c, location loc, const char *format, ...) @@ -1144,21 +1132,10 @@ _PyCompile_Warn(compiler *c, location loc, const char *format, ...) if (msg == NULL) { return ERROR; } - if (PyErr_WarnExplicitObject(PyExc_SyntaxWarning, msg, c->c_filename, - loc.lineno, NULL, NULL) < 0) - { - if (PyErr_ExceptionMatches(PyExc_SyntaxWarning)) { - /* Replace the SyntaxWarning exception with a SyntaxError - to get a more accurate error report */ - PyErr_Clear(); - assert(PyUnicode_AsUTF8(msg) != NULL); - _PyCompile_Error(c, loc, PyUnicode_AsUTF8(msg)); - } - Py_DECREF(msg); - return ERROR; - } + int ret = _PyErr_EmitSyntaxWarning(msg, c->c_filename, loc.lineno, loc.col_offset, + loc.end_lineno, loc.end_col_offset); Py_DECREF(msg); - return SUCCESS; + return ret; } PyObject * diff --git a/Python/errors.c b/Python/errors.c index ad6b7dbef075ccf..f50f416b027241d 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -1850,6 +1850,55 @@ PyErr_SyntaxLocationEx(const char *filename, int lineno, int col_offset) Py_XDECREF(fileobj); } +/* Raises a SyntaxError and returns -1. + * If something goes wrong, a different exception may be raised. + */ + +int +_PyErr_RaiseSyntaxError(PyObject *msg, PyObject *filename, int lineno, int col_offset, + int end_lineno, int end_col_offset) +{ + PyObject *loc_obj = PyErr_ProgramTextObject(filename, lineno); + if (loc_obj == NULL) { + loc_obj = Py_None; + } + PyObject *args = Py_BuildValue("O(OiiOii)", msg, filename, + lineno, col_offset + 1, loc_obj, + end_lineno, end_col_offset + 1); + if (args == NULL) { + goto exit; + } + PyErr_SetObject(PyExc_SyntaxError, args); + exit: + Py_DECREF(loc_obj); + Py_XDECREF(args); + return -1; +} + +/* Emits a SyntaxWarning and returns 0 on success. + If a SyntaxWarning is raised as error, replaces it with a SyntaxError + and returns -1. +*/ +int +_PyErr_EmitSyntaxWarning(PyObject *msg, PyObject *filename, int lineno, int col_offset, + int end_lineno, int end_col_offset) +{ + if (PyErr_WarnExplicitObject(PyExc_SyntaxWarning, msg, filename, + lineno, NULL, NULL) < 0) + { + if (PyErr_ExceptionMatches(PyExc_SyntaxWarning)) { + /* Replace the SyntaxWarning exception with a SyntaxError + to get a more accurate error report */ + PyErr_Clear(); + assert(PyUnicode_AsUTF8(msg) != NULL); + _PyErr_RaiseSyntaxError(msg, filename, lineno, col_offset, + end_lineno, end_col_offset); + } + return -1; + } + return 0; +} + /* Attempt to load the line of text that the exception refers to. If it fails, it will return NULL but will not set an exception. From 6c0d57bac895e072bad59dd86949db4c28f41147 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Wed, 11 Sep 2024 21:00:33 +0100 Subject: [PATCH 2/7] whitespace Co-authored-by: Peter Bierma --- Python/errors.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Python/errors.c b/Python/errors.c index f50f416b027241d..7d3ab11efe06c24 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -1853,7 +1853,6 @@ PyErr_SyntaxLocationEx(const char *filename, int lineno, int col_offset) /* Raises a SyntaxError and returns -1. * If something goes wrong, a different exception may be raised. */ - int _PyErr_RaiseSyntaxError(PyObject *msg, PyObject *filename, int lineno, int col_offset, int end_lineno, int end_col_offset) From 8523c1b67121cbdacafff602856462dc163c8217 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 13 Sep 2024 10:35:44 +0100 Subject: [PATCH 3/7] review comments --- Include/internal/pycore_pyerrors.h | 4 ++-- Python/compile.c | 6 +++--- Python/errors.c | 16 +++++++--------- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/Include/internal/pycore_pyerrors.h b/Include/internal/pycore_pyerrors.h index db61aa181a7b06e..02945f0e71a1451 100644 --- a/Include/internal/pycore_pyerrors.h +++ b/Include/internal/pycore_pyerrors.h @@ -122,8 +122,8 @@ extern PyObject* _PyErr_NoMemory(PyThreadState *tstate); extern int _PyErr_EmitSyntaxWarning(PyObject *msg, PyObject *filename, int lineno, int col_offset, int end_lineno, int end_col_offset); -extern int _PyErr_RaiseSyntaxError(PyObject *msg, PyObject *filename, int lineno, int col_offset, - int end_lineno, int end_col_offset); +extern void _PyErr_RaiseSyntaxError(PyObject *msg, PyObject *filename, int lineno, int col_offset, + int end_lineno, int end_col_offset); PyAPI_FUNC(void) _PyErr_SetString( PyThreadState *tstate, diff --git a/Python/compile.c b/Python/compile.c index 3415cc857c5376e..c70dd84c322ffed 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -1112,10 +1112,10 @@ _PyCompile_Error(compiler *c, location loc, const char *format, ...) if (msg == NULL) { return ERROR; } - int ret = _PyErr_RaiseSyntaxError(msg, c->c_filename, loc.lineno, loc.col_offset, - loc.end_lineno, loc.end_col_offset); + _PyErr_RaiseSyntaxError(msg, c->c_filename, loc.lineno, loc.col_offset, + loc.end_lineno, loc.end_col_offset); Py_DECREF(msg); - return ret; + return ERROR; } /* Emits a SyntaxWarning and returns 0 on success. diff --git a/Python/errors.c b/Python/errors.c index 7d3ab11efe06c24..65e11acde44cf17 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -1850,28 +1850,27 @@ PyErr_SyntaxLocationEx(const char *filename, int lineno, int col_offset) Py_XDECREF(fileobj); } -/* Raises a SyntaxError and returns -1. +/* Raises a SyntaxError. * If something goes wrong, a different exception may be raised. */ -int +void _PyErr_RaiseSyntaxError(PyObject *msg, PyObject *filename, int lineno, int col_offset, int end_lineno, int end_col_offset) { - PyObject *loc_obj = PyErr_ProgramTextObject(filename, lineno); - if (loc_obj == NULL) { - loc_obj = Py_None; + PyObject *text = PyErr_ProgramTextObject(filename, lineno); + if (text == NULL) { + text = Py_None; } PyObject *args = Py_BuildValue("O(OiiOii)", msg, filename, - lineno, col_offset + 1, loc_obj, + lineno, col_offset + 1, text, end_lineno, end_col_offset + 1); if (args == NULL) { goto exit; } PyErr_SetObject(PyExc_SyntaxError, args); exit: - Py_DECREF(loc_obj); + Py_DECREF(text); Py_XDECREF(args); - return -1; } /* Emits a SyntaxWarning and returns 0 on success. @@ -1889,7 +1888,6 @@ _PyErr_EmitSyntaxWarning(PyObject *msg, PyObject *filename, int lineno, int col_ /* Replace the SyntaxWarning exception with a SyntaxError to get a more accurate error report */ PyErr_Clear(); - assert(PyUnicode_AsUTF8(msg) != NULL); _PyErr_RaiseSyntaxError(msg, filename, lineno, col_offset, end_lineno, end_col_offset); } From 4a007f4c12c4c0dce1e911242aed8248298bf2c2 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Fri, 13 Sep 2024 14:44:07 +0100 Subject: [PATCH 4/7] Py_NewRef --- Python/errors.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/errors.c b/Python/errors.c index 65e11acde44cf17..10068d68e57828c 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -1859,7 +1859,7 @@ _PyErr_RaiseSyntaxError(PyObject *msg, PyObject *filename, int lineno, int col_o { PyObject *text = PyErr_ProgramTextObject(filename, lineno); if (text == NULL) { - text = Py_None; + text = Py_NewRef(Py_None); } PyObject *args = Py_BuildValue("O(OiiOii)", msg, filename, lineno, col_offset + 1, text, From 763b250cd7bdc79ffebdaa778a38f21f38c70298 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 16 Sep 2024 11:57:17 +0100 Subject: [PATCH 5/7] increment cols in caller --- Python/compile.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index c70dd84c322ffed..209cc90582c6f86 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -1112,8 +1112,8 @@ _PyCompile_Error(compiler *c, location loc, const char *format, ...) if (msg == NULL) { return ERROR; } - _PyErr_RaiseSyntaxError(msg, c->c_filename, loc.lineno, loc.col_offset, - loc.end_lineno, loc.end_col_offset); + _PyErr_RaiseSyntaxError(msg, c->c_filename, loc.lineno, loc.col_offset + 1, + loc.end_lineno, loc.end_col_offset + 1); Py_DECREF(msg); return ERROR; } From d5687372e00d011416e324b2f0f83ee3c42c5ce7 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 16 Sep 2024 14:21:10 +0100 Subject: [PATCH 6/7] increment cols in caller --- Python/errors.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/errors.c b/Python/errors.c index 10068d68e57828c..29249ac41c63466 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -1862,8 +1862,8 @@ _PyErr_RaiseSyntaxError(PyObject *msg, PyObject *filename, int lineno, int col_o text = Py_NewRef(Py_None); } PyObject *args = Py_BuildValue("O(OiiOii)", msg, filename, - lineno, col_offset + 1, text, - end_lineno, end_col_offset + 1); + lineno, col_offset, text, + end_lineno, end_col_offset); if (args == NULL) { goto exit; } From c679776f462ba0ccf45064345559333056713cb0 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 16 Sep 2024 14:26:10 +0100 Subject: [PATCH 7/7] increment cols in caller - one more place --- Python/compile.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 6b5ccd5e90c8275..b024dc43038af65 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -1132,8 +1132,8 @@ _PyCompile_Warn(compiler *c, location loc, const char *format, ...) if (msg == NULL) { return ERROR; } - int ret = _PyErr_EmitSyntaxWarning(msg, c->c_filename, loc.lineno, loc.col_offset, - loc.end_lineno, loc.end_col_offset); + int ret = _PyErr_EmitSyntaxWarning(msg, c->c_filename, loc.lineno, loc.col_offset + 1, + loc.end_lineno, loc.end_col_offset + 1); Py_DECREF(msg); return ret; }