GeezerVison is showing github.com — The new repl sometimes does not show the error location when `SyntaxError` is raised · Issue #121804 · python/cpython
The new repl sometimes does not show the error location when SyntaxError is raised #121804Labels interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)topic-replRelated to the interactive shellRelated to the interactive shelltype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error DescriptionBug report
Bug description:
When I run def f[Foo, Foo](): ... from a file, I get a nice error message including the location of the error:
File ".../test.py", line 1
def f[Foo, Foo](): ...
^^^
SyntaxError: duplicate type parameter 'Foo'However, when I run this in the new repl, the error location is not shown:
>>> def f[Foo, Foo](): ...
File "<python-input-0>", line 1
SyntaxError: duplicate type parameter 'Foo' The same thing happens with this error as well:
>>> def f(x, x): ...
File "<python-input-0>", line 1
SyntaxError: duplicate argument 'x' in function definition The missing location seems to be the case for these two errors only as other errors do show it:
>>> def f(42): ...
File "<unknown>", line 1
def f(42): ...
^^
SyntaxError: invalid syntaxWhat's interesting is that for the first two errors the file shows up as <python-input-0> while for the last one it's <unknown>. This could be related to the last error coming from the parser while the previous errors come from symtable:
|
if ((o = PyDict_GetItemWithError(dict, mangled))) { |
|
val = PyLong_AS_LONG(o); |
|
if ((flag & DEF_PARAM) && (val & DEF_PARAM)) { |
|
/* Is it better to use 'mangled' or 'name' here? */ |
|
PyErr_Format(PyExc_SyntaxError, DUPLICATE_ARGUMENT, name); |
|
SET_ERROR_LOCATION(st->st_filename, loc); |
|
goto error; |
|
} |
|
if ((flag & DEF_TYPE_PARAM) && (val & DEF_TYPE_PARAM)) { |
|
PyErr_Format(PyExc_SyntaxError, DUPLICATE_TYPE_PARAM, name); |
|
SET_ERROR_LOCATION(st->st_filename, loc); |
|
goto error; |
|
} |
|
val |= flag; |
|
} |
Though it looks like that the error location is being set with SET_ERROR_LOCATION..
CPython versions tested on:
CPython main branch
Operating systems tested on:
Linux
Linked PRs
Activityskirpichev commented on Jul 16, 2024 This issue is valid for basic repl as well:
$ PYTHON_BASIC_REPL=1 ./python
Python 3.14.0a0 (heads/main:bf74db731b, Jul 15 2024, 11:06:00) [GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> def f[Foo, Foo](): ...
...
File "<stdin>-0", line 1
SyntaxError: duplicate type parameter 'Foo'
>>> def f(x, x): ...
...
File "<stdin>-1", line 1
SyntaxError: duplicate argument 'x' in function definition
sobolevn commented on Jul 16, 2024 These errors happen in different places (parser vs symtable):
// symtable.c:1425
PyErr_Format(PyExc_SyntaxError, DUPLICATE_ARGUMENT, name);
SET_ERROR_LOCATION(st->st_filename, loc); So, I guess it is not related to the new repl. skirpichev commented on Jul 16, 2024 Exceptions metadata differ when compile() has valid filename argument wrt fake filename like <input>. It seems, the only way to workaround this is using a temporary file to dump code:
>>> def f(x, x): ...
File "/tmp/tmpuygzaco0", line 1
def f(x, x): ...
^
SyntaxError: duplicate argument 'x' in function definition
POC diff
diff --git a/Lib/_pyrepl/console.py b/Lib/_pyrepl/console.py
index a8d3f52034..0df16c2b3d 100644
--- a/Lib/_pyrepl/console.py
+++ b/Lib/_pyrepl/console.py
@@ -168,6 +168,12 @@ def showtraceback(self):
super().showtraceback(colorize=self.can_colorize)
def runsource(self, source, filename="<input>", symbol="single"):
+ import tempfile
+ import os
+ file = tempfile.NamedTemporaryFile('w+', delete=False)
+ file.write(source)
+ file.close()
+ filename = file.name
try:
tree = ast.parse(source)
except (SyntaxError, OverflowError, ValueError):
@@ -189,11 +195,15 @@ def runsource(self, source, filename="<input>", symbol="single"):
f" top-level 'await' and run background asyncio tasks."
)
self.showsyntaxerror(filename)
+ os.unlink(filename)
return False
except (OverflowError, ValueError):
self.showsyntaxerror(filename)
+ os.unlink(filename)
return False
+ os.unlink(filename)
+
if code is None:
return True
sobolevn commented on Jul 16, 2024 There are multiple systems / setups where temporary files could not be created:
- readonly mounts
- permissions
- etc
Plus, I am not sure that users expect to see File "/tmp/tmpuygzaco0", line 1, I think that it should be a bit more complicated:
- We can try to create and open this temp file, if not - fallback to default behaviour
- If it is opened, then still show an old
<input> tag as a visible filename
There's also a question about how to append / rewrite / recreate this file on new statements.
Storing state is hard :( tomasr8 commented on Jul 16, 2024 imo fixing the core issue with the metadata is probably better than using a temp file for the reasons @sobolevn mentioned, but I don't know enough about the code to tell how hard that would be.. skirpichev commented on Jul 16, 2024
There are multiple systems / setups where temporary files could not be created
Yes, this is not easy:)
Plus, I am not sure that users expect to see File "/tmp/tmpuygzaco0", line 1
I think this can be fixed.
On another hand, we have almost all required metadata for exception in it's attributes:
>>> try:
... compile("def f(x,x): ...\n", "spam.py", "single")
... except Exception as e:
... print(e.lineno, e.offset)
...
1 9The text attribute is empty.
So, we can provide source as an optional kwarg for showsyntaxerror() method. Given this, we can "fix" empty text attribute of the exception. With this patch (see details below):
>>> def f(x, x): ...
File "<python-input-0>", line 1
def f(x, x): ...
^
SyntaxError: duplicate argument 'x' in function definition
>>> def f[Foo, Foo](): ...
File "<python-input-1>", line 1
def f[Foo, Foo](): ...
^^^
SyntaxError: duplicate type parameter 'Foo'
POC patch
diff --git a/Lib/_pyrepl/console.py b/Lib/_pyrepl/console.py
index a8d3f52034..1eda92d49c 100644
--- a/Lib/_pyrepl/console.py
+++ b/Lib/_pyrepl/console.py
@@ -161,8 +161,8 @@ def __init__(
super().__init__(locals=locals, filename=filename, local_exit=local_exit) # type: ignore[call-arg]
self.can_colorize = _colorize.can_colorize()
- def showsyntaxerror(self, filename=None):
- super().showsyntaxerror(colorize=self.can_colorize)
+ def showsyntaxerror(self, filename=None, **kwargs):
+ super().showsyntaxerror(colorize=self.can_colorize, **kwargs)
def showtraceback(self):
super().showtraceback(colorize=self.can_colorize)
@@ -188,7 +188,7 @@ def runsource(self, source, filename="<input>", symbol="single"):
f"Try the asyncio REPL ({python} -m asyncio) to use"
f" top-level 'await' and run background asyncio tasks."
)
- self.showsyntaxerror(filename)
+ self.showsyntaxerror(filename, source=source)
return False
except (OverflowError, ValueError):
self.showsyntaxerror(filename)
diff --git a/Lib/code.py b/Lib/code.py
index a55fced070..78bcdcdca5 100644
--- a/Lib/code.py
+++ b/Lib/code.py
@@ -64,7 +64,7 @@ def runsource(self, source, filename="<input>", symbol="single"):
code = self.compile(source, filename, symbol)
except (OverflowError, SyntaxError, ValueError):
# Case 1
- self.showsyntaxerror(filename)
+ self.showsyntaxerror(filename, source=source)
return False
if code is None:
@@ -107,6 +107,7 @@ def showsyntaxerror(self, filename=None, **kwargs):
"""
colorize = kwargs.pop('colorize', False)
+ source = kwargs.pop('source', "")
type, value, tb = sys.exc_info()
sys.last_exc = value
sys.last_type = type
@@ -124,6 +125,8 @@ def showsyntaxerror(self, filename=None, **kwargs):
value = SyntaxError(msg, (filename, lineno, offset, line))
sys.last_exc = sys.last_value = value
if sys.excepthook is sys.__excepthook__:
+ if source:
+ value.text = source.splitlines()[value.lineno-1]
lines = traceback.format_exception_only(type, value, colorize=colorize)
self.write(''.join(lines))
else:
If this is not too hackish way to deal with the issue - I'll make a pr.
Edit:
Similar fix is possible for basic repl.
POC patch for basic repl
diff --git a/Python/pythonrun.c b/Python/pythonrun.c
index ce7f194e92..43e12acb0e 100644
--- a/Python/pythonrun.c
+++ b/Python/pythonrun.c
@@ -283,6 +283,19 @@ PyRun_InteractiveOneObjectEx(FILE *fp, PyObject *filename,
_PyArena_Free(arena);
Py_DECREF(main_module);
if (res == NULL) {
+ PyThreadState *tstate = _PyThreadState_GET();
+ PyObject *exc = tstate->current_exception;
+ if ((PyObject *)Py_TYPE(exc) == PyExc_SyntaxError) {
+ /* fix "text" attribute */
+ PyObject *xs = PyUnicode_Splitlines(interactive_src, 1);
+ PyObject *ln = PyObject_GetAttr(exc, &_Py_ID(lineno));
+ PyObject *line = PyList_GetItem(xs, PyLong_AsLong(ln) - 1);
+ Py_DECREF(ln);
+ Py_INCREF(line);
+ Py_DECREF(xs);
+ PyObject_SetAttr(exc, &_Py_ID(text), line);
+ Py_DECREF(line);
+ }
return -1;
}
Py_DECREF(res); skirpichev commented on Jul 17, 2024 skirpichev commented on Aug 18, 2024 pablogsal commented on Aug 18, 2024 Reacted by Sergey B Kirpichev skirpichev commented on Aug 21, 2024 @pablogsal, here is a new pr: #123202
Still marked as a draft, because it crashes on debug builds. gdb shows it happens in PyUnicode_Check(interactive_src).
Details
Python 3.14.0a0 (heads/syntaxerr-location-basicrepl-121804-dirty:99f7d742a9, Aug 21 2024, 14:26:0) [GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> def f(x, x): ...
...
Program received signal SIGSEGV, Segmentation fault.
ensure_unicode (obj=obj@entry=<unknown at remote 0x7ffff713f700>) at ./Include/object.h:763
763 return ((flags & feature) != 0);
(gdb) bt
#0 ensure_unicode (obj=obj@entry=<unknown at remote 0x7ffff713f700>) at ./Include/object.h:763
#1 0x0000555555726951 in PyUnicode_Splitlines (string=<unknown at remote 0x7ffff713f700>, keepends=keepends@entry=1)
at Objects/unicodeobject.c:10193
#2 0x000055555581e4fd in PyRun_InteractiveOneObjectEx (fp=fp@entry=0x7ffff7eb4a80 <_IO_2_1_stdin_>,
filename=filename@entry='<stdin>', flags=flags@entry=0x7fffffffd580) at Python/pythonrun.c:290
#3 0x000055555581ff97 in _PyRun_InteractiveLoopObject (fp=fp@entry=0x7ffff7eb4a80 <_IO_2_1_stdin_>,
filename=filename@entry='<stdin>', flags=flags@entry=0x7fffffffd580) at Python/pythonrun.c:131
#4 0x0000555555820430 in _PyRun_AnyFileObject (fp=fp@entry=0x7ffff7eb4a80 <_IO_2_1_stdin_>, filename=filename@entry='<stdin>',
closeit=closeit@entry=0, flags=flags@entry=0x7fffffffd580) at Python/pythonrun.c:71
#5 0x00005555558204e9 in PyRun_AnyFileExFlags (fp=0x7ffff7eb4a80 <_IO_2_1_stdin_>, filename=filename@entry=0x5555558b3886 "<stdin>",
closeit=closeit@entry=0, flags=flags@entry=0x7fffffffd580) at Python/pythonrun.c:98
#6 0x0000555555845b36 in pymain_run_stdin (config=config@entry=0x555555b5f358 <_PyRuntime+199800>) at Modules/main.c:571
#7 0x000055555584615e in pymain_run_python (exitcode=exitcode@entry=0x7fffffffd624) at Modules/main.c:699
#8 0x00005555558463f4 in Py_RunMain () at Modules/main.c:775
#9 0x000055555584646b in pymain_main (args=args@entry=0x7fffffffd680) at Modules/main.c:805
#10 0x0000555555846531 in Py_BytesMain (argc=<optimized out>, argv=<optimized out>) at Modules/main.c:829
#11 0x00005555555d5842 in main (argc=<optimized out>, argv=<optimized out>) at ./Programs/python.c:15
It could be reproduced on the current master with:
diff --git a/Python/pythonrun.c b/Python/pythonrun.c
index ce7f194e92..2601c9268c 100644
--- a/Python/pythonrun.c
+++ b/Python/pythonrun.c
@@ -282,6 +282,10 @@ PyRun_InteractiveOneObjectEx(FILE *fp, PyObject *filename,
PyObject *res = run_mod(mod, filename, main_dict, main_dict, flags, arena, interactive_src, 1);
_PyArena_Free(arena);
Py_DECREF(main_module);
+ if (interactive_src) {
+ printf("float? - %d\n", PyFloat_Check(interactive_src));
+ printf("unicode? - %d\n", PyUnicode_Check(interactive_src));
+ }
if (res == NULL) {
return -1;
}Works on non-debug builds (prints 0 and then 1) and crashes on the first check for debug builds. I think it's a bug, but not with my code: the interactive_src is a PyObject, type checks on it shouldn't crash interpreter.
Should I report new issue? pablogsal commented on Aug 21, 2024
Should I report new issue?
It's not a bug, the interactive_src object it's own by the arena and you are freeing the arena two lines before that, which in turn frees the object because it's only own by thee arena. If you incref the interactive_src object before freeing the arena it won't crash.
In debug mode crashes because the allocator has a special code that writes 0xfdddddddddd to the object so it forces crashes, but in release mode the memory is marked as freed but still has not being used so you don't crash immediately. skirpichev commented on Aug 22, 2024 Thanks for help! Now pr is ready for review. picnixz commented on Aug 28, 2024 I'm reopening this issue (and add the labels) until we've merged the second PR (just for tracking purposes). Reacted by Sergey B Kirpichev added a commit that references this issue on Sep 3, 2024gh-121804: always show error location for SyntaxError's in basic repl (…
MetadataMetadatainterpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)topic-replRelated to the interactive shellRelated to the interactive shelltype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error DevelopmentNo branches or pull requestsIssue actions
|