Skip to content

The new repl sometimes does not show the error location when SyntaxError is raised #121804

Description

@tomasr8

Bug 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 syntax

What'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:

cpython/Python/symtable.c

Lines 1421 to 1435 in 6522f0e

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

Activity

  1. skirpichev commented on Jul 16, 2024

    @skirpichev
    Member

    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
    
  2. sobolevn commented on Jul 16, 2024

    @sobolevn
    Member

    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.

  3. skirpichev commented on Jul 16, 2024

    @skirpichev
    Member

    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
     
  4. sobolevn commented on Jul 16, 2024

    @sobolevn
    Member

    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 :(

  5. tomasr8 commented on Jul 16, 2024

    @tomasr8
    MemberAuthor

    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..

  6. skirpichev commented on Jul 16, 2024

    @skirpichev
    Member

    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 9

    The 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);
  7. added 2 commits that reference this issue on Jul 17, 2024
  8. skirpichev commented on Jul 17, 2024

    @skirpichev
    Member

    pr is ready for review: #121886

  9. skirpichev commented on Aug 18, 2024

    @skirpichev
    Member

    cc @pablogsal, maybe you could take look on pr?

  10. pablogsal commented on Aug 18, 2024

    @pablogsal
    Member

    Will do today

  11. added a commit that references this issue on Aug 19, 2024
  12. 8 remaining items

  13. added a commit that references this issue on Aug 21, 2024
  14. skirpichev commented on Aug 21, 2024

    @skirpichev
    Member

    @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?

  15. pablogsal commented on Aug 21, 2024

    @pablogsal
    Member

    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.

  16. skirpichev commented on Aug 22, 2024

    @skirpichev
    Member

    Thanks for help! Now pr is ready for review.

  17. added a commit that references this issue on Aug 22, 2024
  18. added 2 commits that reference this issue on Aug 26, 2024
  19. picnixz commented on Aug 28, 2024

    @picnixz
    Member

    I'm reopening this issue (and add the labels) until we've merged the second PR (just for tracking purposes).

  20. added
    topic-replRelated to the interactive shell
    interpreter-core(Objects, Python, Grammar, and Parser dirs)
    on Aug 28, 2024
  21. added a commit that references this issue on Sep 3, 2024
  22. added a commit that references this issue on Sep 3, 2024
  23. added a commit that references this issue on Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    interpreter-core(Objects, Python, Grammar, and Parser dirs)topic-replRelated to the interactive shelltype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions