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

gh-101865: Deprecate co_lnotab from code objects as per PEP 626 #101866

Merged
merged 10 commits into from
Apr 3, 2023

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Feb 13, 2023

I still have to fix these lines:

cpython/Misc/gdbinit

Lines 60 to 61 in 6ef6915

set $__sz = ((PyVarObject *)$__co->co_lnotab)->ob_size/2
set $__p = (unsigned char *)((PyBytesObject *)$__co->co_lnotab)->ob_sval

But, since I have 0 experience with gdb it might take some time.

@sobolevn
Copy link
Member Author

I've pushed my naive attempt to fix gdb: since it only needs a current line f->f_lineno seems like a tool for the job. (again, this is my very first time working with gdb)

@sobolevn
Copy link
Member Author

@markshannon friendly ping :)

@sobolevn sobolevn changed the title gh-101865: Remove deprecated co_lnotab from code objects as per PEP626 gh-101865: Deprecate co_lnotab from code objects as per PEP626 Feb 21, 2023
@sobolevn
Copy link
Member Author

sobolevn commented Feb 22, 2023

Failures do not seem related 🤔

@arhadthedev
Copy link
Member

arhadthedev commented Feb 25, 2023

test_implied_dirs_performance issue was fixed in gh-102225 so merging main should help.

@sobolevn
Copy link
Member Author

@markshannon do you have time to re-review this, please? :)

@sobolevn
Copy link
Member Author

sobolevn commented Mar 18, 2023

@carljm maybe you can have a look? :)

Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in general.
We aren't obliged to remove co_lnotab in 3.14, but we can. Perhaps change the "will"s to "may"s.

Not sure what to do about Misc/gdbinit though.

Doc/whatsnew/3.12.rst Outdated Show resolved Hide resolved
Misc/gdbinit Outdated Show resolved Hide resolved
Objects/codeobject.c Outdated Show resolved Hide resolved
@markshannon
Copy link
Member

Perhaps we should discuss the future of Misc/gdbinit in another issue, and not worry about it here.

@sobolevn
Copy link
Member Author

@markshannon thanks a lot for your review and for all your hard work on CPython!

  1. I've addressed all your comments
  2. I've opened a new issue about Misc/gdbinit: What to do with Misc/gdbinit? #102809

@hugovk
Copy link
Member

hugovk commented Mar 18, 2023

Shall we add a .. deprecated-removed: 3.12 3.14 in the main docs, somewhere near https://docs.python.org/3.12/reference/datamodel.html?highlight=co_lnotab ?

@hugovk hugovk changed the title gh-101865: Deprecate co_lnotab from code objects as per PEP626 gh-101865: Deprecate co_lnotab from code objects as per PEP 626 Mar 18, 2023
@sobolevn
Copy link
Member Author

sobolevn commented Mar 19, 2023

I think that we can add "is deprecated since 3.12 and may be removed in 3.14".
But this directive won't work, because it is a raw text:

  1. https://github.com/python/cpython/blame/3adb23a17d25e36bd80874e860835182d851623f/Doc/reference/datamodel.rst#L965
  2. https://github.com/python/cpython/blame/3adb23a17d25e36bd80874e860835182d851623f/Doc/reference/datamodel.rst#L992

@hugovk
Copy link
Member

hugovk commented Mar 19, 2023

I think that we can add "is deprecated since 3.12 and may be removed in 3.14".

Sure, let's do something like that.

@sobolevn
Copy link
Member Author

gdbinit was removed in #102854 🎉

@sobolevn
Copy link
Member Author

sobolevn commented Apr 3, 2023

@markshannon I've solved the merge conflicts. I hope that this is ready to be merged when you have the time :)

@ambv ambv merged commit 2a72125 into python:main Apr 3, 2023
gaogaotiantian pushed a commit to gaogaotiantian/cpython that referenced this pull request Apr 8, 2023
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
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.

6 participants