GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-45711: Remove type and traceback from exc_info by iritkatriel · Pull Request #30122 · python/cpython

Conversation

@iritkatriel

@iritkatriel iritkatriel commented Dec 15, 2021 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

https://bugs.python.org/issue45711

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
iritkatriel added 6 commits December 14, 2021 18:08
@iritkatriel
bpo-45711: Do not PUSH/POP traceback to the stack as part of exc_info
10687bd
@iritkatriel
bpo-45711: remove exc_traceback from _PyErr_StackItem
37c7f12
@iritkatriel
removed exc_type from stack
c5c23bd
@iritkatriel
remove exc_type from exc_info
28ce9d2
@iritkatriel
update the except* opcodes
8ee6d84
@iritkatriel
Merge remote-tracking branch 'upstream/main' into 45711-exc_info_just…
3306286
…_value
@iritkatriel
iritkatriel requested review from 1st1, asvetlov and markshannon as code owners December 15, 2021 14:39
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Dec 15, 2021
@bedevere-bot bedevere-bot added the awaiting core review label Dec 15, 2021
@iritkatriel iritkatriel changed the title 45711 exc info just value bpo-45711: Remove type and traceback from exc_info Dec 15, 2021
@iritkatriel
iritkatriel requested a review from gvanrossum December 15, 2021 14:40
@iritkatriel
val --> exc in comment
9b04160
arhadthedev
arhadthedev reviewed Dec 15, 2021
View reviewed changes
Comment thread Include/internal/pycore_pyerrors.h Resolved

Uh oh!

There was an error while loading. Please reload this page.

@iritkatriel iritkatriel added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 15, 2021
@bedevere-bot

bedevere-bot commented Dec 15, 2021

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @iritkatriel for commit 9b04160 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 15, 2021
iritkatriel and others added 3 commits December 15, 2021 15:10
@iritkatriel
add to what's new, because this change breaks things like cython
4cd5af8
@blurb-it
📜🤖 Added by blurb_it.
a72556e
@iritkatriel
delete duplicate news file
787393b
@markshannon markshannon self-assigned this Dec 15, 2021
@iritkatriel iritkatriel mentioned this pull request Dec 15, 2021
bpo-45711: Do not push/pop traceback on stack as part of exc_info #29968
Closed
iritkatriel added 2 commits December 16, 2021 10:57
@iritkatriel
PEP7 // formatting
458ad26
@iritkatriel
Merge remote-tracking branch 'upstream/main' into 45711-exc_info_just…
b2db87b
…_value
markshannon
markshannon reviewed Dec 16, 2021
View reviewed changes
Comment thread Include/internal/pycore_pyerrors.h Outdated


/* number of items stored in the stack for each exception */
#define PY_EXC_INFO_STACK_SIZE 1

@markshannon markshannon Dec 16, 2021

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Is this necessary now?
I don't see why we would ever want to increase it, and I don't see how we could ever decrease it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@iritkatriel iritkatriel Dec 16, 2021

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The name is probably weird now, but I think it helps readability to have a macro. If we remove this we should find another way to make the code (particularly in compile.c) clear about where exceptions are showing up. Figuring out where to use this macro was the hardest part of making this change (because when you get it wrong it just segfaults before you finish make regen-all).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@iritkatriel iritkatriel Dec 17, 2021

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

On second thought there is no point in a macro for this unless we all agree on some kind of convention because it probably won't be updated correctly when people make changes. So I'll remove it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
markshannon
markshannon reviewed Dec 16, 2021
View reviewed changes
Comment thread Lib/test/test_sys.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

markshannon
markshannon reviewed Dec 16, 2021
View reviewed changes
Comment thread Python/ceval.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@markshannon

markshannon commented Dec 16, 2021

Copy link
Copy Markdown
Member

This is great. 🍾

Removes code, should make any exception handling code faster and reduces stack consumption; the perfect combination.
A couple of very minor quibbles, otherwise looks really good.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@iritkatriel
use Py_Clear
01f9036
@tacaswell tacaswell mentioned this pull request Dec 16, 2021
[BUG] incompatibility with CPython main branch cython/cython#4499
Closed
iritkatriel added 2 commits December 16, 2021 23:46
@iritkatriel
3 --> 1 in comments
ba82085
@iritkatriel
remove PY_EXC_INFO_STACK_SIZE
80b1e90
arhadthedev
arhadthedev requested changes Dec 17, 2021
View reviewed changes
Comment thread Python/compile.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@iritkatriel @arhadthedev
Fix indent
550f6f9
Co-authored-by: Oleg Iarygin <[email protected]>
@iritkatriel

iritkatriel commented Dec 17, 2021

Copy link
Copy Markdown
Member Author

The windows test failure is unrelated

File "D:\a\cpython\cpython\Lib\test\support_init_.py", line 197, in _force_run
1232
return func(*args)
1233
^^^^^^^^^^^
1234
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'D:\a\cpython\cpython\build\test_python_4272�\test_python_worker_516�'

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@markshannon

markshannon commented Dec 17, 2021

Copy link
Copy Markdown
Member

Excellent.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@markshannon
markshannon merged commit 396b583 into python:main Dec 17, 2021
@bedevere-bot bedevere-bot removed the awaiting core review label Dec 17, 2021
@iritkatriel
iritkatriel deleted the 45711-exc_info_just_value branch January 13, 2022 15:33
@Araneidae Araneidae mentioned this pull request May 16, 2023
Build failure on Python3.11 DiamondLightSource/cothread#36
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markshannon markshannon markshannon left review comments
@arhadthedev arhadthedev arhadthedev requested changes
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner
@gvanrossum gvanrossum Awaiting requested review from gvanrossum

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@markshannon markshannon

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@iritkatriel @bedevere-bot @markshannon @arhadthedev @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.