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 — Move exc state to generator. Fixes bpo-25612 by markshannon · Pull Request #1773 · python/cpython

Conversation

@markshannon

@markshannon markshannon commented May 23, 2017 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Moves the exception state from the frame to the coroutine. (Coroutine in the CS sense, which includes generators, coroutines and thread). This cleans up the code, removing about 60 lines, and fixes https://bugs.python.org/issue25612.

https://bugs.python.org/issue25612

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@the-knights-who-say-ni

the-knights-who-say-ni commented May 23, 2017

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@the-knights-who-say-ni the-knights-who-say-ni added the CLA not signed label May 23, 2017
@mention-bot

mention-bot commented May 23, 2017

Copy link
Copy Markdown

@markshannon, thanks for your PR! By analyzing the history of the files in this pull request, we identified @benjaminp, @serhiy-storchaka and @Yhg1s to be potential reviewers.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@1st1
1st1 self-requested a review May 23, 2017 22:12
@markshannon

markshannon commented May 23, 2017

Copy link
Copy Markdown
Member Author

I've just linked by bpo user to my github user. Which should satisfy @the-knights-who-say-ni, hopefully.
All reactions

Sorry, something went wrong.

Uh oh!

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

njsmith commented May 23, 2017

Copy link
Copy Markdown
Contributor

You might also want to check how this affects this strange behavior that @arigo found: https://bugs.python.org/issue28884#msg282532
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
1st1
1st1 reviewed May 23, 2017
View reviewed changes

@1st1 1st1 left a comment

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.

I like this refactoring a lot. I'd go a step further and refactor tstate->*curexc_ too as well as add a couple of helper macros (not defined in limited API).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Objects/genobject.c Outdated

@1st1 1st1 May 23, 2017

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.

Maybe we should add a macro PyExcState_CLEAR (and _VISIT)?

Sorry, something went wrong.

Uh oh!

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

@markshannon markshannon May 24, 2017

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.

Done (although I used inline functions)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/pystate.h Outdated

@1st1 1st1 May 23, 2017

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.

Given that this is already quite an invasive change, maybe replace curexc_* fields with curexc_state?

Sorry, something went wrong.

Uh oh!

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

@markshannon markshannon May 24, 2017

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.

PyExcState structs form a stack (hence the "previous" field), but there is only one current exception.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/pystate.h Outdated

@1st1 1st1 May 23, 2017

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.

Can we have exc_info, exc_state and curexc_* defined next to each other with comments explaining how we use each one of them? Since you've already spent time to figure this stuff out anyways.

Sorry, something went wrong.

Uh oh!

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

@markshannon markshannon May 24, 2017

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.

Done

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Objects/genobject.c Outdated

@1st1 1st1 May 23, 2017

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.

If we had a PyExcState_CLEAR macro we could use it here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@markshannon
markshannon force-pushed the move-exc-state-to-generator branch from e0b55d4 to e19057d Compare May 23, 2017 23:34
@markshannon

markshannon commented May 23, 2017

Copy link
Copy Markdown
Member Author

@njsmith It does indeed fix https://bugs.python.org/issue28884#msg282532. I've added it as a test case.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@the-knights-who-say-ni the-knights-who-say-ni added CLA signed and removed CLA not signed labels May 24, 2017
1st1
1st1 reviewed May 25, 2017
View reviewed changes
Comment thread Include/pystate.h Outdated

@1st1 1st1 May 25, 2017 •
edited
Loading

Uh oh!

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

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.

There are many lines that are longer than 79 (see PEP 7). Could you please fix this? </nitpicking>

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
1st1
1st1 reviewed May 25, 2017
View reviewed changes
Comment thread Python/errors.c Outdated

@1st1 1st1 May 25, 2017

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.

Another nit, per PEP 7 we now want to use { even for blocks with one statement.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@1st1

1st1 commented May 25, 2017

Copy link
Copy Markdown
Member

Aside from a few code formatting nits the patch looks good. I just want to test it on my machine before giving it a green light; will do that tomorrow or the day after.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
1st1
1st1 reviewed May 26, 2017
View reviewed changes
Comment thread Objects/genobject.c Outdated

@1st1 1st1 May 26, 2017

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.

Another minor nit: please rename to PyExcState_Clear.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@markshannon
markshannon force-pushed the move-exc-state-to-generator branch 2 times, most recently from 0cc764b to 800150c Compare June 11, 2017 11:04
larryhastings
larryhastings reviewed Jul 15, 2017
View reviewed changes
Comment thread Include/pystate.h Outdated

@larryhastings larryhastings Jul 15, 2017

Copy link
Copy Markdown
Contributor

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.

Should this comment be left verbatim like this? The comment applied when these fields were part of the frame object, but they've been removed from the frame object completely. I don't claim to understand the patch completely but I assume the use of these fields have changed somewhat--otherwise what's the point of shuffling the data around?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
larryhastings
larryhastings reviewed Jul 15, 2017
View reviewed changes
Comment thread Include/pystate.h Outdated

@larryhastings larryhastings Jul 15, 2017

Copy link
Copy Markdown
Contributor

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.

Would you consider a longer name here? I always remember MvL saying "avoid abbreviations in identifiers, they are mystifying to non-English speakers". "PyExceptionState" isn't that awful is it?

Sorry, something went wrong.

Uh oh!

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

@serhiy-storchaka serhiy-storchaka Oct 11, 2017

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.

If this name is not public, it should be underscored.

A short name would LGTM, but the PyExc prefix is used only for exception classes. The PyException prefix is used for manipulating individual exception objects (PyExceptionClass_Check, PyException_GetTraceback). All other names related to exceptions and error handling have prefix PyErr.

Sorry, something went wrong.

Uh oh!

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

@pitrou pitrou Oct 11, 2017

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.

PyExcState sounds ok to me.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
1st1 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@Mariatta Mariatta added needs rebase and removed needs rebase labels Oct 9, 2017
@1st1
1st1 requested a review from pitrou October 10, 2017 23:16
@1st1

1st1 commented Oct 10, 2017

Copy link
Copy Markdown
Member

@pitrou Antoine, could you please take a look? I'm inclined to merge this.

@markshannon Could you please rebase and add a NEWS entry? Would be cool if you could address the few review nits, but if you don't have time I can merge this as is and fix them myself.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@1st1
1st1 requested a review from serhiy-storchaka October 10, 2017 23:18
serhiy-storchaka
serhiy-storchaka reviewed Oct 11, 2017
View reviewed changes
Comment thread Include/pyerrors.h Outdated

@serhiy-storchaka serhiy-storchaka Oct 11, 2017

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.

PyAPI_FUNC()

Sorry, something went wrong.

Uh oh!

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

@markshannon markshannon Oct 22, 2017

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.

This isn't intended to be an API function. It is only used in errors.c and sysmodule.c

Sorry, something went wrong.

Uh oh!

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

@pitrou pitrou Oct 22, 2017

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.

Even for private functions we still use PyAPI_FUNC by convention (such as _PyErr_SetKeyError just above).

Sorry, something went wrong.

Uh oh!

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

@markshannon markshannon Oct 22, 2017

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.

But it isn't meant to be part of an API. What does PyAPI_FUNC mean?

Sorry, something went wrong.

Uh oh!

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

@1st1 1st1 Oct 22, 2017

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.

Even for private functions we still use PyAPI_FUNC by convention (such as _PyErr_SetKeyError just above).

See _PyCoro_GetAwaitableIter and _PyGen_yf etc. I agree with @markshannon here, we don't really need PyAPI_FUNC.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/pystate.h Outdated

@serhiy-storchaka serhiy-storchaka Oct 11, 2017

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.

If this name is not public, it should be underscored.

A short name would LGTM, but the PyExc prefix is used only for exception classes. The PyException prefix is used for manipulating individual exception objects (PyExceptionClass_Check, PyException_GetTraceback). All other names related to exceptions and error handling have prefix PyErr.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Objects/genobject.c Outdated

@serhiy-storchaka serhiy-storchaka Oct 11, 2017

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.

Py_LOCAL_INLINE(int)?

Sorry, something went wrong.

Uh oh!

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

@markshannon markshannon Oct 22, 2017

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.

Now that we support C99, I think we ought to use standard C where possible.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Objects/genobject.c Outdated

@serhiy-storchaka serhiy-storchaka Oct 11, 2017

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.

Move { on a separate line.

If PyExcState_traverse is private and local use the all-lowercase-letters style.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Objects/genobject.c Outdated

@serhiy-storchaka serhiy-storchaka Oct 11, 2017

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.

Move static inline void (or Py_LOCAL_INLINE(int)?) and { on separate lines.

If PyExcState_Clear is private and local use the all-lowercase-letters style.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/errors.c Outdated

@serhiy-storchaka serhiy-storchaka Oct 11, 2017

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.

Move PyExcState * and { on separate lines.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/errors.c Outdated

@serhiy-storchaka serhiy-storchaka Oct 11, 2017

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.

Move { on a separate line (new PEP 7 rule for multiline conditions).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/pystate.c Outdated

@serhiy-storchaka serhiy-storchaka Oct 11, 2017

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.

Py_VerboseFlag only is used for import related tracing.

Sorry, something went wrong.

Uh oh!

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

@markshannon markshannon Oct 22, 2017

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.

This is consistent with https://github.com/python/cpython/blob/master/Python/pystate.c#L434. I'm not sure what is the ideal behaviour, but it ought to be consistent.

Sorry, something went wrong.

Uh oh!

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

@serhiy-storchaka serhiy-storchaka Oct 22, 2017

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.

Then that code should be fixed too.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/pystate.c Outdated

@serhiy-storchaka serhiy-storchaka Oct 11, 2017

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.

Can PyErr_WarnEx() or PySys_WriteStderr() be used here?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/errors.c Outdated

@serhiy-storchaka serhiy-storchaka Oct 11, 2017

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.

The result of this function is read-only, and exc_previous is not used. Wouldn't be better to return just three objects like in PyErr_GetExcInfo()? This would make the PyExcState structure more private.

Sorry, something went wrong.

Uh oh!

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

@markshannon markshannon Oct 22, 2017

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.

If I made it return three objects, it would be the same as PyErr_GetExcInfo().
_PyErr_GetExcInfo() is provided as a way to handle the type, value, traceback trio in a reasonably modular fashion, without to much overhead.
As it is only used in errors.c and sysmodule.c, it is effectively private.

Sorry, something went wrong.

Uh oh!

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

@serhiy-storchaka serhiy-storchaka Oct 22, 2017

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.

If it would be the same as PyErr_GetExcInfo(), we should just use PyErr_GetExcInfo().

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
pitrou
pitrou reviewed Oct 11, 2017
View reviewed changes
Comment thread Python/pystate.c Outdated

@pitrou pitrou Oct 11, 2017

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.

Add a comment here?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@pitrou

pitrou commented Oct 11, 2017

Copy link
Copy Markdown
Member

This looks good to me on the principle. It needs adressing @serhiy-storchaka 's comments, and also fixing the merge conflicts.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@1st1

1st1 commented Oct 11, 2017

Copy link
Copy Markdown
Member

@markshannon Mark, will you be able to work on this in the following couple of weeks? If not, I can help resolve code review comments.
All reactions

Sorry, something went wrong.

Uh oh!

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

markshannon commented Oct 12, 2017

Copy link
Copy Markdown
Member Author

Not right now, but in the next week or so, yes.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markshannon Mark Shannon
Move exception state information from frame objects to coroutine (gen…
adde498 
…erator/thread) object where it belongs.
markshannon added 4 commits October 22, 2017 11:31
@markshannon Mark Shannon
Add tests to verify fix of bpo-25612 and case B6 of bpo-28884
207ff6c
@markshannon Mark Shannon
Factor out clear/traverse for exc_state struct.
52acc3c
@markshannon Mark Shannon
Make sure that previous exc-state in thread is initialized properly.
f2ff96e
@markshannon Mark Shannon
Make code conform to PEP-7
963aea3
@markshannon
markshannon force-pushed the move-exc-state-to-generator branch from 800150c to 415121e Compare October 22, 2017 12:04
markshannon added 2 commits October 22, 2017 13:18
@markshannon Mark Shannon
Change names and formatting to conform to PEP-7 and make explicit tha…
22b1e82 
…t exception 'state' forms a stack.
@markshannon Mark Shannon
Add blurb entry for new exception state handling.
e5ffc67
@markshannon
markshannon force-pushed the move-exc-state-to-generator branch from 7929841 to e5ffc67 Compare October 22, 2017 12:19
1st1
1st1 approved these changes Oct 22, 2017
View reviewed changes
@bedevere-bot bedevere-bot added the awaiting merge label Oct 22, 2017
@pitrou
Right-justify C macro
d3b7efb
pitrou
pitrou approved these changes Oct 22, 2017
View reviewed changes
@pitrou

pitrou commented Oct 22, 2017

Copy link
Copy Markdown
Member

@markshannon, do you have push rights or would you like someone else to merge this PR?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka
serhiy-storchaka reviewed Oct 22, 2017
View reviewed changes
Comment thread Include/pystate.h

struct _exc_stackitem *exc_previous;

} _PyErr_StackItem;

@serhiy-storchaka serhiy-storchaka Oct 22, 2017

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.

Inconsistent names _exc_stackitem and _PyErr_StackItem.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Objects/genobject.c Outdated
PyErr_Restore(error_type, error_value, error_traceback);
}

static inline void exc_state_clear(_PyErr_StackItem *exc_state)

@serhiy-storchaka serhiy-storchaka Oct 22, 2017

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.

Move static inline void on a separate line.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/errors.c Outdated
Py_XDECREF(oldtraceback);
}

_PyErr_StackItem *_PyErr_GetTopmostException(PyThreadState *tstate)

@serhiy-storchaka serhiy-storchaka Oct 22, 2017

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.

Move _PyErr_StackItem * on a separate line.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/errors.c Outdated

@serhiy-storchaka serhiy-storchaka Oct 22, 2017

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.

If it would be the same as PyErr_GetExcInfo(), we should just use PyErr_GetExcInfo().

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@serhiy-storchaka

serhiy-storchaka commented Oct 22, 2017

Copy link
Copy Markdown
Member

Is the problem I described on the tracker resolved?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
markshannon added 2 commits October 22, 2017 21:13
@markshannon
More renaming and PEP-7 compliance
36ea52f
@markshannon
Update implementation of 'raise' to scan exception stack.
cba130e
@markshannon

markshannon commented Oct 22, 2017

Copy link
Copy Markdown
Member Author

@serhiy-storchaka I've just fixed it.
@pitrou I don't have push rights.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@pitrou
pitrou merged commit ae3087c into python:master Oct 22, 2017
@bedevere-bot bedevere-bot removed the awaiting merge label Oct 22, 2017
@markshannon
markshannon deleted the move-exc-state-to-generator branch October 23, 2017 07:57
@1st1

1st1 commented Oct 23, 2017

Copy link
Copy Markdown
Member

Thanks a lot, Mark, for looking into this.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
tacaswell added a commit to tacaswell/cython that referenced this pull request Oct 28, 2017
@tacaswell
FIX: account for change in how exception information is stored
fdf4451 
CPython change how the exception information is stored internally in
3.7a3.

This simply adds fences based on python version around how to access
the objects.

Xref:

python/cpython@ae3087c
https://bugs.python.org/issue25612
python/cpython#1773
This was referenced Oct 28, 2017
Internals of exceptions on ThreadState _ts struct have moved cython/cython#1955
Closed
FIX: account for change in how exception information is stored cython/cython#1956
Merged
scoder pushed a commit to cython/cython that referenced this pull request Oct 28, 2017
@tacaswell @scoder
FIX: account for change in how exception information is stored
2d33924 
CPython change how the exception information is stored internally in
3.7a3.

This simply adds fences based on python version around how to access
the objects.

Xref:

python/cpython@ae3087c
https://bugs.python.org/issue25612
python/cpython#1773
@snaury snaury mentioned this pull request Dec 3, 2017
Doesn't compile with Python 3.7 due to tstate changes python-greenlet/greenlet#123
Closed
akruis added a commit to stackless-dev/stackless that referenced this pull request Oct 13, 2018
@akruis
Stackless issue #149: Move exc state from thread to tasklet
245a802 
bpo-25612 (python#1773) moves the exception state information from frame
object to coroutine (generator/thread) object where it belongs.
As a consequence Stackless moves the exception state information for
non-current tasklets from thread-state to the tasklet. This changes the
pickle format of frame, tasklet and generator objects.
The commit adds three test cases.
@ghost ghost mentioned this pull request Mar 29, 2019
cothread will not build using Python 3.7 DiamondLightSource/cothread#9
Closed
@indrora indrora mentioned this pull request Jun 15, 2019
Fails to build under Python 3.7 seomoz/url-py#63
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments
@larryhastings larryhastings larryhastings left review comments
@1st1 1st1 1st1 approved these changes
@pitrou pitrou pitrou approved these changes

Uh oh!

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

Assignees

No one assigned

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.

10 participants

@markshannon @the-knights-who-say-ni @mention-bot @njsmith @1st1 @pitrou @serhiy-storchaka @larryhastings @Mariatta @bedevere-bot
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 01402520 — 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.