[issue18533] Avoid error from repr() of recursive dictview

[Serhiy Storchaka]

Change by Serhiy Storchaka :


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

[issue18533] Avoid error from repr() of recursive dictview

[miss-islington]

miss-islington  added the comment:


New changeset fbf7aac36bd1017bc87964b5d17dce0e101ff2d6 by Miss Islington (bot) 
in branch '3.6':
bpo-18533: Avoid RecursionError from repr() of recursive dictview (GH-4823)
https://github.com/python/cpython/commit/fbf7aac36bd1017bc87964b5d17dce0e101ff2d6


--
nosy: +miss-islington

___
Python tracker 

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

[issue18533] Avoid error from repr() of recursive dictview

[Ben North]

Ben North  added the comment:

Terry in msg310785:
> As I expected from the fact that Ben once did a separate 2.7
> version, the auto backport for 2.7 failed.  Ben, since you know what
> code changes are needed, can you prepare a backport (cherry pick)
> PR?

Yes, that's right, there were some differences between the approaches
in Py2 and Py3.  I've created PR5357 for the 2.7 backport.

--

___
Python tracker 

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

[issue18533] Avoid error from repr() of recursive dictview

[Ben North]

Change by Ben North :


--
pull_requests: +5203
stage: needs patch -> patch review

___
Python tracker 

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

[issue18533] Avoid error from repr() of recursive dictview

[Terry J. Reedy]

Terry J. Reedy  added the comment:

Thank you for checking and merging.  In the future, we will be reminded to 
change '#' to 'GH-' in the title.  I only found out a couple of days ago that 
this is desired.

David, in msg193651, and Raymond, in msg193779, already suggested backporting.  
I added the 'needs backport' tags.

The auto backport for 3.6 worked.  You might want to checked the diff as well 
as test results.

As I expected from the fact that Ben once did a separate 2.7 version,  the auto 
backport for 2.7 failed.  Ben, since you know what code changes are needed, can 
you prepare a backport (cherry pick) PR?

--
assignee:  -> orsenthil
stage: patch review -> needs patch
versions: +Python 2.7, Python 3.6

___
Python tracker 

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

[issue18533] Avoid error from repr() of recursive dictview

[Roundup Robot]

Change by Roundup Robot :


--
pull_requests: +5196

___
Python tracker 

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

[issue18533] Avoid error from repr() of recursive dictview

[Senthil Kumaran]

Senthil Kumaran  added the comment:

I merged this one.  All the requirements were met.

@Terry / @ Cheryl - could you please review if I did it right?  Should this be 
backported to other Python versions?

--
resolution:  -> fixed
versions: +Python 3.7 -Python 2.7, Python 3.3, Python 3.4

___
Python tracker 

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

[issue18533] Avoid error from repr() of recursive dictview

[Senthil Kumaran]

Senthil Kumaran  added the comment:


New changeset d7773d92bd11640a8c950d6c36a9cef1cee36f96 by Senthil Kumaran 
(bennorth) in branch 'master':
bpo-18533: Avoid RecursionError from repr() of recursive dictview (#4823)
https://github.com/python/cpython/commit/d7773d92bd11640a8c950d6c36a9cef1cee36f96


--
nosy: +orsenthil

___
Python tracker 

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

[issue18533] Avoid error from repr() of recursive dictview

[Terry J. Reedy]

Terry J. Reedy  added the comment:

OK, test_dictviews and test_ordered_dict now pass for me, and Appveyor already 
ran entire suite and says OK to all.

--

___
Python tracker 

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

[issue18533] Avoid error from repr() of recursive dictview

[Terry J. Reedy]

Terry J. Reedy  added the comment:

Hold the failure comment.  I forgot that I have to recompile when patch changes 
.c files.

--

___
Python tracker 

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

[issue18533] Avoid error from repr() of recursive dictview

[Terry J. Reedy]

Terry J. Reedy  added the comment:

Not ready to merge.
1. We need the Appveyer test to pass or special action from the gateway gods.
2. After a month, the PR needed to be updated and retested anyway.  I pulled, 
updated, pushed the update (which will trigger Travis and AV), and tested on my 
Windows machine. The two new tests fail.  See PR.
3. Someone has to sell assign and merge when ready.

--

___
Python tracker 

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

[issue18533] Avoid error from repr() of recursive dictview

[Cheryl Sabella]

Cheryl Sabella  added the comment:

It looks like Serhiy approved the PR on 12/13.  Is it ready to merge?  Thanks!

--

___
Python tracker 

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

[issue18533] Avoid error from repr() of recursive dictview

[Ben North]

Ben North  added the comment:

PR4823 created as per msg308086.

--

___
Python tracker 

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

[issue18533] Avoid error from repr() of recursive dictview

[Roundup Robot]

Change by Roundup Robot :


--
pull_requests: +4713
stage: test needed -> patch review

___
Python tracker 

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

[issue18533] Avoid error from repr() of recursive dictview

[STINNER Victor]

STINNER Victor  added the comment:

See also bpo-32137.

--
nosy: +serhiy.storchaka

___
Python tracker 

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

[issue18533] Avoid error from repr() of recursive dictview

[Cheryl Sabella]

Cheryl Sabella  added the comment:

Ben, would you be interested in making a Github pull request against the 
current master for your patch?  It appears there was interest in moving forward 
with this and it is still occurring on 3.7, although it now raises a 
RecursionError.

>>> d = {}
>>> d[1] = d.keys()
>>> d
{1: dict_keys([1])}
>>> d[2] = d.values()
>>> d
Traceback (most recent call last):
  File "", line 1, in 
RecursionError: maximum recursion depth exceeded while getting the repr of an 
object

--
nosy: +csabella

___
Python tracker 

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

[issue18533] Avoid error from repr() of recursive dictview

[Ben North]

Ben North added the comment:

Is anything further needed from me before this can be reviewed?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18533
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18533] Avoid error from repr() of recursive dictview

[Terry J. Reedy]

Terry J. Reedy added the comment:

Please visit
http://www.python.org/psf/contrib/
http://www.python.org/psf/contrib/contrib-form/
and submit a Contributor Agreement. This process is complete when '*' appears 
after your name here, as with mine and others.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18533
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18533] Avoid error from repr() of recursive dictview

[Ben North]

Changes by Ben North b...@redfrontdoor.org:


Added file: http://bugs.python.org/file31089/against-9bf89c909bd4-3.3-1.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18533
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18533] Avoid error from repr() of recursive dictview

[Ben North]

Ben North added the comment:

I'll attach fresh patches, one against latest 2.7 and one against
latest 3.3 (unchanged apart from headers from previous patch).

Both branches pass

./python -m test.regrtest -R20:30 test_dictviews

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18533
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18533] Avoid error from repr() of recursive dictview

[Ben North]

Changes by Ben North b...@redfrontdoor.org:


Added file: http://bugs.python.org/file31088/against-6e1dd1ce95b8-2.7-1.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18533
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18533] Avoid error from repr() of recursive dictview

[Raymond Hettinger]

Raymond Hettinger added the comment:

+1 for backporting the fix.

--
nosy: +rhettinger

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18533
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18533] Avoid error from repr() of recursive dictview

[Ben North]

Ben North added the comment:

New patch, fixing nit noted in msg193624:

non-error-recursive-dictview-3.3-1.patch

Otherwise same as previous

non-error-recursive-dictview-3.3.patch

--
Added file: 
http://bugs.python.org/file31025/non-error-recursive-dictview-3.3-1.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18533
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18533] Avoid error from repr() of recursive dictview

[R. David Murray]

R. David Murray added the comment:

I'm in favor of treating this a bugfix.  It is a bug for repr to raise a 
recursion error, IMO.

--
nosy: +r.david.murray

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18533
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18533] Avoid error from repr() of recursive dictview

[Ben North]

Ben North added the comment:

New patch including tests attached, against 3.3.  Terry Reedy's example
above now gives

 d = {}
 d[1] = d.keys()
 d[2] = d.values()
 d
{1: dict_keys([1, 2]), 2: dict_values([dict_keys([1, 2]), ...])}

which I think is preferable.

Summary of patch:

dictobject.c:

dictview_repr() now uses a Py_ReprEnter() / Py_ReprLeave() pair to
check for recursion, and produces ... if so.  I think I've got the
behaviour correct in the case that PySequence_List() fails, but
couldn't find a way to trigger this in testing.

test_dictviews.py:

test_recursive_repr() checks for the correct string (containing
...) rather than a RuntimeError.

test_deeply_nested_repr() is a new function, aiming to replace the
original test_recursive_repr().  It checks that a RuntimeError is
raised in the case of a non-recursive but deeply nested structure.

Under --with-pydebug,

./python -m test.regrtest -R20:30 test_dictviews

passes.

--
Added file: 
http://bugs.python.org/file31022/non-error-recursive-dictview-3.3.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18533
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18533] Avoid error from repr() of recursive dictview

[STINNER Victor]

STINNER Victor added the comment:

non-error-recursive-dictview-3.3.patch: small nit, initialize result to NULL, 
instead of setting it to NULL only in case of error. It makes the code more 
readable and it is the common coding style to handle errors.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18533
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18533] Avoid error from repr() of recursive dictview

[STINNER Victor]

Changes by STINNER Victor victor.stin...@gmail.com:


--
nosy: +haypo

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18533
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18533] Avoid error from repr() of recursive dictview

[Ben North]

New submission from Ben North:

#18019 noted the following crash in earlier 2.7:

 d={}
 d[42]=d.viewvalues()
 d
segmentation fault

This issue has been fixed; the behaviour now is that a RuntimeError is
produced for a recursive dictionary view:

 d={}
 d[42]=d.viewvalues()
 d # (output line-broken:)
{42: Traceback (most recent call last):
  File stdin, line 1, in module
RuntimeError: maximum recursion depth exceeded
  while getting the repr of a list

Before finding this, though, I'd investigated and made a patch which
produces a similar ... output to a recursive dictionary.  Reworking
against current 2.7, the behaviour would be:

 x={}
 x[42]=x
 x # existing behaviour for dictionaries:
{42: {...}}

 d={}
 d[42]=d.viewvalues()
 d # new behaviour:
{42: dict_values([...])}
 d[43]=d.viewitems()
 d # (output line-broken:)
{42: dict_values([..., dict_items([(42, ...), (43, ...)])]),
 43: dict_items([(42, dict_values([..., ...])), (43, ...)])}

Attached is the patch, against current 2.7 branch.  If there is interest
in applying this, I will create a proper patch (changelog entry, fix to
Lib/test/test_dictviews.py, etc.).

On python-dev, Gregory Smith noted:

Given that the RuntimeError fix has been released, your proposed
... behavior is arguably a new feature so I'd only expect this to
make sense for consideration in 3.4, not 2.7.  (if accepted at all)

[http://mail.python.org/pipermail/python-dev/2013-July/127489.html]

so I have marked for consideration for versions 2.7 and 3.4.

--
components: Interpreter Core
files: non-error-recursive-dictview.patch
keywords: patch
messages: 193570
nosy: bennorth
priority: normal
severity: normal
status: open
title: Avoid error from repr() of recursive dictview
type: behavior
versions: Python 2.7, Python 3.4
Added file: http://bugs.python.org/file31019/non-error-recursive-dictview.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18533
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18533] Avoid error from repr() of recursive dictview

[Terry J. Reedy]

Terry J. Reedy added the comment:

Fixing 3.3 is more likely that 3.4. I can view RuntimeError as a bug less 
obnoxious than  crash, but others may differ. Ultimately, the release managers 
can decide. This is definitely appropriate for 3.4, so please add tests. If the 
patch is for 2.7, please do one for 3.3 or 3.4.

For 3.x, d.keys() is not an issue, but d.values() is.

 d = {}
 d[1] = d.keys()
 d
{1: dict_keys([1])}
 d[2] = d.values()
 d
Traceback (most recent call last):
  File stdin, line 1, in module
RuntimeError: maximum recursion depth exceeded while getting the repr of a list

--
nosy: +terry.reedy
stage:  - test needed
versions: +Python 3.3

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18533
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18533] Avoid error from repr() of recursive dictview

[Jan Kaliszewski]

Jan Kaliszewski added the comment:

As I wrote on the list -- IMHO it's still a bug (even though not so painful as 
segfault) that should also be fixed in 2.7 and 3.2/3.3.  In other cases (such 
as `d={}; d[42]=d; repr(d)`) Python does its best to avoid an error -- why in 
this case (`d={}; d[42]=d.Py2.x:viewvalues(); repr(d)`) should it raise an 
exception? IMHO it's an obvious oversight in implementation, not a feature that 
anybody would expect.

--
nosy: +zuo

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18533
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com