[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +23389
pull_request: https://github.com/python/cpython/pull/24582

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Raphaël M]

Raphaël M  added the comment:

Thank you very much!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Inada Naoki]

Inada Naoki  added the comment:

> I've tried writing some Python code to reproduce this bug, but I'm unable to 
> -- I should be missing something. Is there a simple snippet showing the 
> issue? 


Note that this is a bug from long ago.  Why this bug had lived long is it can 
not happen in regular cases.  So it is difficult to reproduce.

See PR 2.  _csv module is changed to use PyDict_GetItemWithError.
Let's try it on Python 3.7.

Python 3.7.6 (default, Dec 30 2019, 19:38:28)
[Clang 11.0.0 (clang-1100.0.33.16)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> class S(str):
...   def __hash__(self):
... raise MemoryError
...
>>> import _csv
>>> _csv.Dialect(S("excel"))
Traceback (most recent call last):
  File "", line 1, in 
_csv.Error: unknown dialect

You can see the MemoryError is suppressed.  Let's try it on Python 3.8.

$ python3
Python 3.8.1 (default, Jan  6 2020, 16:02:33)
 (snip)
>>> _csv.Dialect(S("excel"))
Traceback (most recent call last):
  File "", line 1, in 
  File "", line 3, in __hash__
MemoryError

You can see the MemoryError is not suppressed.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Raphaël M]

Raphaël M  added the comment:

Any pointer would also be welcome, and a piece of code showing the bug would 
help me a lot. Thank you.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Raphaël M]

Raphaël M  added the comment:

I stumbled upon this issue while reading Python 3.8 and this made me curious.

I've tried writing some Python code to reproduce this bug, but I'm unable to -- 
I should be missing something. Is there a simple snippet showing the issue? 


Also, the changelog states that "The CPython interpreter can swallow exceptions 
in some circumstances". Are there other documented cases of those circumstances?

--
nosy: +raphaelm

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

There are few occurrences of PyDict_GetItem(), PyDict_GetItemString() and 
_PyDict_GetItemId() in cases where they are unlikely failed. These cases will 
be considered in separate issues.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:


New changeset 7a0630c530121725136526a88c49589b54da6492 by Serhiy Storchaka in 
branch 'master':
Add a What's New entry for bpo-35459. (GH-12706)
https://github.com/python/cpython/commit/7a0630c530121725136526a88c49589b54da6492


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
pull_requests: +12630
stage: resolved -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Inada Naoki]

Inada Naoki  added the comment:

Serhiy, can this issue be closed?

--
nosy: +inada.naoki

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Josh Rosenberg]

Josh Rosenberg  added the comment:

#36110 was closed as a duplicate; the superseder is #36109 (which has been 
fixed). The change should still be documented, just in case anyone gets bitten 
by it.

--
nosy: +josh.r

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[STINNER Victor]

STINNER Victor  added the comment:

> It seems like the change introduced a regression: bpo-36110.

While the test has been fixed, IMHO we need to better document this subtle 
behavior change since it can be surprising.

The test failed because PyObject_GetAttr() no longer ignores exceptions when 
getting the attribute from the type dictionary. It's a significant change 
compared to Python 3.7.

Right now, the change has not even a NEWS entry, whereas it's a backward 
incompatible change!

Don't get my wrong: the change is correct, we don't want to ignore arbitrary 
exceptions, it's just a matter of documentation.

--
nosy: +pablogsal
resolution: fixed -> 
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[STINNER Victor]

STINNER Victor  added the comment:

It seems like the change introduced a regression: bpo-36110.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:


New changeset a24107b04c1277e3c1105f98aff5bfa3a98b33a0 by Serhiy Storchaka in 
branch 'master':
bpo-35459: Use PyDict_GetItemWithError() instead of PyDict_GetItem(). (GH-2)
https://github.com/python/cpython/commit/a24107b04c1277e3c1105f98aff5bfa3a98b33a0


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Eric Snow]

Eric Snow  added the comment:

Thanks, Serhiy.  While the benchmark suite is our best tool available for 
measuring performance, I'm not sure what slowdown is significant in those 
results.

@Victor, any thoughts?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Eric requested to run the benchmark suite. Here are results. I do not know how 
to interpret them. Likely all differences are random.

--
Added file: https://bugs.python.org/file48137/pydict-getitem-compare.txt

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
nosy: +eric.snow, rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Most of changes are straightforward. Just replaced PyDict_GetItem*() with 
PyDict_GetItem*WithError() and added the check for PyErr_Occurred(). 
PyDict_GetItemString() with constant argument was replaced with 
_PyDict_GetItemIdWithError() for performance.

Some code was left unchanged. This was mostly in files where errors are very 
and error checking is not performed or errors are silenced in any case 
(Python/compile.c, Python/symtable.c, Objects/structseq.c, etc). These cases 
needed separate issues.

The most non-trivial change is in Objects/typeobject.c. The check for 
duplicated descriptors (in add_methods(), add_members() and add_getset()) was 
moved after creating the descriptor object. This improves performance by 
avoiding to create a temporary string objects. Duplicate descriptor names is a 
very uncommon case -- there were only two cases in the stdlib (besides tests), 
and one of them already is fixed (PR 11053).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Opened issue35461 for documenting flaws of PyDict_GetItem().

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
keywords: +patch
pull_requests: +10341
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[STINNER Victor]

STINNER Victor  added the comment:

My previous attempt: bpo-20615.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35459] Use PyDict_GetItemWithError() instead of PyDict_GetItem()

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
title: Use PyDict_GetItemWithError() with PyDict_GetItem() -> Use 
PyDict_GetItemWithError() instead of PyDict_GetItem()

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com