Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-46042: Improve SyntaxError locations in the symbol table #30059

Merged
merged 3 commits into from Dec 11, 2021

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Dec 11, 2021

@cfbolz
Copy link
Contributor

cfbolz commented Dec 11, 2021

Looks good to me! I like the macros and just having symtable_add_def always take a position.

@cfbolz
Copy link
Contributor

cfbolz commented Dec 11, 2021

I think we could be even more general. It seems the other two remaining uses of ste_lineno for error locations are also wrong:

[x for x in range(10)
        if (yield) == 5]
def f():
    from math import *

@pablogsal
Copy link
Member Author

pablogsal commented Dec 11, 2021

The second one is a bit more complex because we would need to identify which of the comprehension generators have a 'yield' inside, but that happens downstream from us in the visitors.

@pablogsal
Copy link
Member Author

pablogsal commented Dec 11, 2021

From the second example I have submitted another commit

@cfbolz
Copy link
Contributor

cfbolz commented Dec 11, 2021

PyPy has the equivalent of an extra ste_type in the _block_type enum that's used for comprehensions. Then you could produce the error when visiting the yield?

@pablogsal
Copy link
Member Author

pablogsal commented Dec 11, 2021

PyPy has the equivalent of an extra ste_type in the _block_type enum that's used for comprehensions. Then you could produce the error when visiting the yield?

We would need to also know the comprehension type to raise properly, which is a bit messy. Do you have one extra enum or 4 extra ones?

Edit: I did a prototype but is quite long so I will do this in another PR

@pablogsal
Copy link
Member Author

pablogsal commented Dec 11, 2021

Nevermind, I found a better workaround that is not too inelegant IMHO

@cfbolz
Copy link
Contributor

cfbolz commented Dec 11, 2021

Nice, that's a good approach!

@pablogsal pablogsal merged commit 59435ee into python:main Dec 11, 2021
12 checks passed
@pablogsal pablogsal deleted the bpo-46042 branch Dec 11, 2021
@miss-islington
Copy link
Contributor

miss-islington commented Dec 11, 2021

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒🤖

@bedevere-bot
Copy link

bedevere-bot commented Dec 11, 2021

GH-30064 is a backport of this pull request to the 3.10 branch.

pablogsal added a commit to miss-islington/cpython that referenced this pull request Dec 11, 2021
…H-30059)

(cherry picked from commit 59435ee)

Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>
@bedevere-bot
Copy link

bedevere-bot commented Dec 11, 2021

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot ARM64 macOS 3.x has failed when building commit 59435ee.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/725/builds/575) and take a look at the build logs.
  4. Check if the failure is related to this commit (59435ee) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/725/builds/575

Summary of the results of the build (if available):

== Tests result: ENV CHANGED ==

410 tests OK.

10 slowest tests:

  • test_concurrent_futures: 3 min 36 sec
  • test_multiprocessing_spawn: 2 min 31 sec
  • test_multiprocessing_forkserver: 1 min 46 sec
  • test_unparse: 1 min 30 sec
  • test_asyncio: 1 min 18 sec
  • test_tokenize: 1 min 13 sec
  • test_logging: 57.2 sec
  • test_lib2to3: 53.7 sec
  • test_capi: 53.0 sec
  • test_io: 46.9 sec

1 test altered the execution environment:
test_ftplib

17 tests skipped:
test_dbm_gnu test_devpoll test_epoll test_gdb test_ioctl
test_msilib test_multiprocessing_fork test_ossaudiodev test_spwd
test_startfile test_tix test_tk test_ttk_guionly test_winconsoleio
test_winreg test_winsound test_zipfile64

Total duration: 9 min 28 sec

Click to see traceback logs
Traceback (most recent call last):
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/threading.py", line 1031, in _bootstrap_inner
    self.run()
    ^^^^^^^^^^
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/test/test_ftplib.py", line 298, in run
    asyncore.loop(timeout=0.1, count=1)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/asyncore.py", line 214, in loop
    poll_fun(timeout, map)
    ^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/asyncore.py", line 157, in poll
    read(obj)
    ^^^^^^^^^
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/asyncore.py", line 94, in read
    obj.handle_error()
    ^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/test/test_ftplib.py", line 421, in handle_error
    raise Exception
    ^^^^^^^^^^^^^^^
Exception
k


Traceback (most recent call last):
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/asyncore.py", line 90, in read
    obj.handle_read_event()
    ^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/test/test_ftplib.py", line 384, in handle_read_event
    self._do_ssl_handshake()
    ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/test/test_ftplib.py", line 345, in _do_ssl_handshake
    self.socket.do_handshake()
    ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/ssl.py", line 1346, in do_handshake
    self._sslobj.do_handshake()
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^
ssl.SSLZeroReturnError: TLS/SSL connection has been closed (EOF) (_ssl.c:998)


Traceback (most recent call last):
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/multiprocessing/resource_tracker.py", line 209, in main
    cache[rtype].remove(name)
    ^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: '/psm_46c6e6df'


Traceback (most recent call last):
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/multiprocessing/resource_tracker.py", line 209, in main
    cache[rtype].remove(name)
    ^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: '/psm_8c999b84'

pablogsal added a commit that referenced this pull request Dec 12, 2021
… (GH-30064)

(cherry picked from commit 59435ee)

Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>

Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants