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

PEP-654: warn that leaf_generator recycles tracebacks #2109

Merged
merged 2 commits into from
Oct 16, 2021

Conversation

iritkatriel
Copy link
Member

No description provided.

pep-0654.rst Outdated
Comment on lines 474 to 477
# Note: the traceback segments are shallow-copied
# into tbs so they can be shared by multiple leaf
# exceptions. Make a deep copy if your use case
# requires that.
Copy link
Member

Choose a reason for hiding this comment

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

Huh, this is actually ambiguous (I hadn't realized this before): the singly-linked list of Traceback objects is mutable (you could set any item's tb_next and mutate the chain) and in addition the tbs list is reused by each iteration through the generator.

I'm not particularly worried about people mutating the Traceback objects, but somehow your comment appears to refer to them. To describe the reuse of tbs, maybe reformulate the note like this?

Note: the list returned (tbs) is reused by each iteration through the generator.
Make a copy of your use case holds on to it beyond the current iteration.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah I see, yes. I am a bit concerned also about people concatenating the tb chunks of tbs to make it look like one traceback.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

I wouldn't worry too much about people mutating the linked lists of Traceback objects -- we don't mutate those (even if the traceback is bubbled up, we only append new Traceback objects to the front of the list), so it's a lesser concern than people just storing a reference to the yielded list, which is definitely going to give them a hard-to-debug problem. (We could solve the latter by returning a copy of the list, but that seems unperformant in the common case.)

@iritkatriel
Copy link
Member Author

I wouldn't worry too much about people mutating the linked lists of Traceback objects -- we don't mutate those (even if the traceback is bubbled up, we only append new Traceback objects to the front of the list), so it's a lesser concern than people just storing a reference to the yielded list, which is definitely going to give them a hard-to-debug problem. (We could solve the latter by returning a copy of the list, but that seems unperformant in the common case.)

The common case is you don't iterate at all. I hope nobody will use this function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants