Skip to content

gh-106033: Get rid of new occurrences of PyDict_GetItem and PyObject_HasAttr - #106034

Merged
serhiy-storchaka merged 2 commits into
python:mainfrom
serhiy-storchaka:no-use-PyDict_GetItem-and-PyObject_HasAttr
Jun 23, 2023
Merged

gh-106033: Get rid of new occurrences of PyDict_GetItem and PyObject_HasAttr#106034
serhiy-storchaka merged 2 commits into
python:mainfrom
serhiy-storchaka:no-use-PyDict_GetItem-and-PyObject_HasAttr

Conversation

@serhiy-storchaka

@serhiy-storchakaserhiy-storchaka commented Jun 23, 2023•
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

These functions are broken by design because they discard any exceptions raised inside, including MemoryError and KeyboardInterrupt. They should not be used in new code.

@serhiy-storchaka
pythongh-106033: Get rid of new occurrences of PyDict_GetItem and PyO…
c593435
…bject_HasAttr
These functions are broken by design because they discard any exceptions raised
inside, including MemoryError and KeyboardInterrupt. They should not be
used in new code.
@bedevere-botbedevere-bot mentioned this pull request Jun 23, 2023
Closed
iritkatriel
iritkatriel reviewed Jun 23, 2023
View reviewed changes
Comment threadObjects/exceptions.c
iritkatriel
iritkatriel reviewed Jun 23, 2023
View reviewed changes
Comment threadPython/pythonrun.c
}
if (!PySequence_Check(notes) || PyUnicode_Check(notes) || PyBytes_Check(notes)) {
int res = 0;
res = 0;

@iritkatrieliritkatrielJun 23, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is there a reason not to have this local to the block?

@serhiy-storchakaserhiy-storchakaJun 23, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

It will hide the same named variable in the outer block. It looks errorprone, even if it does not do wrong in this case. The compiler can complain, and I would not blame it on this.

iritkatriel
iritkatriel approved these changes Jun 23, 2023
View reviewed changes

@iritkatrieliritkatriel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@serhiy-storchaka
serhiy-storchaka merged commit 1d33d53 into python:mainJun 23, 2023
@miss-islington

miss-islington commented Jun 23, 2023

Copy link
Copy Markdown
Contributor

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

@miss-islington

miss-islington commented Jun 23, 2023

Copy link
Copy Markdown
Contributor

Sorry @serhiy-storchaka, I had trouble checking out the 3.12 backport branch.
Please retry by removing and re-adding the "needs backport to 3.12" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 1d33d5378058671bfabb6f4d4b5bfd4726973ff9 3.12

@serhiy-storchaka
serhiy-storchaka deleted the no-use-PyDict_GetItem-and-PyObject_HasAttr branch June 23, 2023 17:10
@miss-islington

miss-islington commented Jun 23, 2023

Copy link
Copy Markdown
Contributor

Sorry, @serhiy-storchaka, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 1d33d5378058671bfabb6f4d4b5bfd4726973ff9 3.11

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Jun 23, 2023
@serhiy-storchaka
[3.11]pythongh-106033: Get rid of new occurrences of PyDict_GetItem …
11c3096
…and PyObject_HasAttr (pythonGH-106034)
These functions are broken by design because they discard any exceptions raised
inside, including MemoryError and KeyboardInterrupt. They should not be
used in new code..
(cherry picked from commit 1d33d53)
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@serhiy-storchakaserhiy-storchaka mentioned this pull request Jun 23, 2023
Merged
@bedevere-bot

bedevere-bot commented Jun 23, 2023

Copy link
Copy Markdown

GH-106040 is a backport of this pull request to the 3.11 branch.

@bedevere-botbedevere-bot removed the needs backport to 3.11 only security fixes label Jun 23, 2023
@serhiy-storchakaserhiy-storchaka mentioned this pull request Jun 23, 2023
Merged
@bedevere-bot

bedevere-bot commented Jun 23, 2023

Copy link
Copy Markdown

GH-106041 is a backport of this pull request to the 3.12 branch.

@bedevere-botbedevere-bot removed the needs backport to 3.12 only security fixes label Jun 23, 2023
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Jun 23, 2023
@serhiy-storchaka
[3.12]pythongh-106033: Get rid of new occurrences of PyDict_GetItem …
4224df2
…and PyObject_HasAttr (pythonGH-106034)
These functions are broken by design because they discard any exceptions raised
inside, including MemoryError and KeyboardInterrupt. They should not be
used in new code.
(cherry picked from commit 1d33d53)
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
gpshead pushed a commit that referenced this pull request Jun 24, 2023
@serhiy-storchaka
[3.12]gh-106033: Get rid of new occurrences of PyDict_GetItem and Py… (
9cd3664
#106041)
[3.12] gh-106033: Get rid of new occurrences of PyDict_GetItem and PyObject_HasAttr (GH-106034)
These functions are broken by design because they discard any exceptions raised
inside, including MemoryError and KeyboardInterrupt. They should not be
used in new code.
(cherry picked from commit 1d33d53)
gpshead pushed a commit that referenced this pull request Jun 24, 2023
@serhiy-storchaka
[3.11]gh-106033: Get rid of new occurrences of PyDict_GetItem and Py… (
dbe416b
#106040)
[3.11] gh-106033: Get rid of new occurrences of PyDict_GetItem and PyObject_HasAttr (GH-106034)
These functions are broken by design because they discard any exceptions raised
inside, including MemoryError and KeyboardInterrupt. They should not be
used in new code..
(cherry picked from commit 1d33d53)
@serhiy-storchakaserhiy-storchaka removed their assignment Jul 25, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iritkatrieliritkatrieliritkatriel approved these changes
@markshannonmarkshannonAwaiting requested review from markshannonmarkshannon is a code owner

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@serhiy-storchaka@miss-islington@bedevere-bot@iritkatriel