bpo-42482: remove reference to exc_traceback from TracebackException by iritkatriel · Pull Request #23531 · python/cpython

iritkatriel · 2020-11-27T10:49:29Z

https://bugs.python.org/issue42482

Automerge-Triggered-By: GH:gvanrossum

…ption

gvanrossum

Yeah!

iritkatriel · 2020-11-27T23:13:42Z

Yeah!

Wait, I made a mistake in the refcount test. The TracebackException object is an intermediate, I need to assign it to something.

gvanrossum

Sorry I didn't catch that. :-(

Let me know when you are confident enough that I can merge this.

iritkatriel · 2020-11-28T06:48:36Z

I think it’s ready now.

iritkatriel · 2020-11-28T10:48:56Z

BTW - the faulty test did fail on the old code. It must be that the exc_traceback field was closing a cycle with the test function’s frame or something like that.

iritkatriel · 2020-11-29T23:08:18Z

Actually it might make more sense to move the line that prints the "traceback header" to StackSummary.format(), where the traceback itself is being formatted.

gvanrossum · 2020-11-29T23:16:29Z

Actually it might make more sense to move the line that prints the "traceback header" to StackSummary.format(), where the traceback itself is being formatted.

Except that would change the behavior of TracebackException.format(), which feels like a public(ish) API.

iritkatriel · 2020-11-29T23:28:20Z

Actually it might make more sense to move the line that prints the "traceback header" to StackSummary.format(), where the traceback itself is being formatted.

Except that would change the behavior of TracebackException.format(), which feels like a public(ish) API.

Yes, I'm adding a with_header=False arg to format(). Alternatively, we could leave the header line where it is but under "if self.stack:" and remove the _print_traceback_header boolean.

iritkatriel · 2020-11-29T23:42:22Z

BTW - the faulty test did fail on the old code. It must be that the exc_traceback field was closing a cycle with the test function’s frame or something like that.

This is the cause - the TracebackException was not really intermediate because format() returns a generator:

(Pdb) gc.get_referrers(gc.get_referrers(exc_info[2])[2])
[<traceback.TracebackException object at 0x000001E0D48E3A90>]
(Pdb) gc.get_referrers(gc.get_referrers(gc.get_referrers(exc_info[2])[2])[0])
[<frame at 0x000001E0D2551C90, file 'C:\Users\User\src\cpython\lib\traceback.py', line 616, code format>]
(Pdb) gc.get_referrers(gc.get_referrers(gc.get_referrers(gc.get_referrers(exc_info[2])[2])[0])[0])
[<generator object TracebackException.format at 0x000001E0D49E63C0>]
(Pdb) gc.get_referrers(gc.get_referrers(gc.get_referrers(gc.get_referrers(gc.get_referrers(exc_info[2])[2])[0])[0])[0])
[{'self': <test.test_traceback.TestTracebackException testMethod=test_no_refs_to_exception_and_traceback_objects>, 'exc_info': (<class 'ZeroDivisionError'>, ZeroDivisionError('division by zero'), <traceback object at 0x000001E0D48DE880>), 'refcnt1': 2, 'refcnt2': 3, '_': <generator object TracebackException.format at 0x000001E0D49E63C0>, 'gc': <module 'gc' (built-in)>}]

iritkatriel · 2020-11-30T00:01:58Z

I went for just removing the boolean. At this point moving the header line (as an opt-in) won't make the code clearer than this.

gvanrossum

Okay, though I am curious why the original fixer didn't do it that way...

gvanrossum · 2020-11-30T00:07:40Z

-        if self._print_traceback_header:
+        if self.stack:
            yield 'Traceback (most recent call last):\n'
        yield from self.stack.format()


Maybe indent this line too then?

gvanrossum

Let's sit on this for a day...

miss-islington · 2020-12-01T01:35:28Z

Thanks @iritkatriel for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

bedevere-bot · 2020-12-01T01:35:47Z

GH-23578 is a backport of this pull request to the 3.9 branch.

bedevere-bot · 2020-12-01T01:35:56Z

GH-23579 is a backport of this pull request to the 3.8 branch.

…ythonGH-23531) (cherry picked from commit 427613f) Co-authored-by: Irit Katriel <iritkatriel@yahoo.com>

…H-23531) (cherry picked from commit 427613f) Co-authored-by: Irit Katriel <iritkatriel@yahoo.com>

…ption (GH-23531) (GH-23578) (cherry picked from commit 427613f) Co-authored-by: Irit Katriel <iritkatriel@yahoo.com>

…ythonGH-23531)

the-knights-who-say-ni added the CLA signed label Nov 27, 2020

bedevere-bot added the awaiting review label Nov 27, 2020

bpo-42482: remove reference to exception traceback from TracebackExce…

3233ecd

…ption

iritkatriel force-pushed the bpo-42482 branch from 132b460 to 3233ecd Compare November 27, 2020 16:45

blurb-it Bot and others added 2 commits November 27, 2020 16:46

📜🤖 Added by blurb_it.

5459051

added refcount check to unit tests

920f808

gvanrossum approved these changes Nov 27, 2020

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Nov 27, 2020

iritkatriel added 2 commits November 27, 2020 23:19

create ref to the TracebackException object

16120a1

no need to call format() in refcnt test

47c13ad

gvanrossum approved these changes Nov 28, 2020

View reviewed changes

remove unnecessary boolean

d4fbfb2

gvanrossum reviewed Nov 30, 2020

View reviewed changes

indent stack.format() line as well

1035665

gvanrossum approved these changes Nov 30, 2020

View reviewed changes

gvanrossum changed the title ~~bpo-42482: remove reference to exception traceback from TracebackExce…~~ bpo-42482: remove reference to exc_traceback from TracebackException Nov 30, 2020

gvanrossum added needs backport to 3.8 labels Dec 1, 2020

miss-islington merged commit 427613f into python:master Dec 1, 2020

bedevere-bot removed awaiting merge needs backport to 3.9 labels Dec 1, 2020

bedevere-bot removed the needs backport to 3.8 label Dec 1, 2020

miss-islington added a commit that referenced this pull request Dec 1, 2020

bpo-42482: remove reference to exc_traceback from TracebackException (G…

1cc5c94

…H-23531) (cherry picked from commit 427613f) Co-authored-by: Irit Katriel <iritkatriel@yahoo.com>

miss-islington added a commit that referenced this pull request Dec 4, 2020

[3.9] bpo-42482: remove reference to exc_traceback from TracebackExce…

40b92f1

…ption (GH-23531) (GH-23578) (cherry picked from commit 427613f) Co-authored-by: Irit Katriel <iritkatriel@yahoo.com>

iritkatriel added the type-bug An unexpected behavior, bug, or error label Dec 23, 2020

adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021

bpo-42482: remove reference to exc_traceback from TracebackException (p…

027e6d8

…ythonGH-23531)

iritkatriel deleted the bpo-42482 branch March 16, 2021 13:46

Uh oh!

Conversation

iritkatriel commented Nov 27, 2020 • edited by miss-islington Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

iritkatriel commented Nov 27, 2020

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

iritkatriel commented Nov 28, 2020

Uh oh!

iritkatriel commented Nov 28, 2020

Uh oh!

iritkatriel commented Nov 29, 2020

Uh oh!

gvanrossum commented Nov 29, 2020

Uh oh!

iritkatriel commented Nov 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iritkatriel commented Nov 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iritkatriel commented Nov 30, 2020

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

gvanrossum Nov 30, 2020

Choose a reason for hiding this comment

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington commented Dec 1, 2020

Uh oh!

bedevere-bot commented Dec 1, 2020

Uh oh!

bedevere-bot commented Dec 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

iritkatriel commented Nov 27, 2020 •

edited by miss-islington

Loading

iritkatriel commented Nov 29, 2020 •

edited

Loading

iritkatriel commented Nov 29, 2020 •

edited

Loading