bpo-38316: describe co_stacksize a little bit better by isidentical · Pull Request #16983 · python/cpython

isidentical · 2019-10-29T10:02:06Z

https://bugs.python.org/issue38316

ammaraskar · 2019-11-15T18:47:11Z

hmm, now that I think about this more. I think more appropriate wording would be:

co_stacksize is the largest size the stack will grow to when executing the function;

isidentical · 2019-11-15T22:20:56Z

@ammaraskar can you check it now?

ammaraskar

LGTM with final suggestion

ammaraskar · 2019-11-15T23:19:34Z

@vstinner Would you mind taking a look at this?

vstinner

Objects/frameobject.c allocates an array of code->co_stacksize + code->co_nlocals + ncells + nfrees items. The array size doesn't chagne during the execution of the function, it's not like x86 machine mode which manipulates the stack pointer (SP) register and use PUSH/POP.

:attr:co_stacksize is the required stack size (including local variables)

So this definition sounds wrong to me.

Instead, I suggest this definition:

:attr:`co_stacksize` is the required stack size

=> just remove (including local variables).

bedevere-bot · 2019-12-14T14:51:53Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

isidentical · 2019-12-15T06:41:56Z

I have made the requested changes; please review again

bedevere-bot · 2019-12-15T06:41:59Z

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

miss-islington · 2019-12-15T22:03:02Z

Thanks @isidentical for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

miss-islington · 2019-12-15T22:03:03Z

Thanks @isidentical for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington · 2019-12-15T22:03:08Z

Sorry, @isidentical and @vstinner, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker d587272fe3b0fcad2f23a490e76f9f82ca7d64ef 3.7

miss-islington · 2019-12-15T22:03:10Z

Sorry @isidentical and @vstinner, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker d587272fe3b0fcad2f23a490e76f9f82ca7d64ef 3.8

vstinner · 2019-12-15T22:04:00Z

@isidentical: The automated backport to 3.7 and 3.8 failed. Can you please try to manually backport these changes? Use " cherry_picker d587272 3.8" or "git cherry-pick -x d587272".

vstinner · 2019-12-18T20:45:08Z

Ping @isidentical, tell me if you prefer that I do the backport.

isidentical · 2019-12-19T14:06:19Z

Thanks for the ping @vstinner, I totally forgot backports. Sending them right away

(cherry picked from commit d587272) Co-authored-by: Batuhan Taşkaya <[email protected]>

bedevere-bot · 2019-12-19T14:36:09Z

GH-17660 is a backport of this pull request to the 3.7 branch.

(cherry picked from commit d587272) Co-authored-by: Batuhan Taşkaya <[email protected]>

bedevere-bot · 2019-12-19T14:38:41Z

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

@vstinner

(cherry picked from commit d587272) Co-authored-by: Batuhan Taşkaya <[email protected]> https://bugs.python.org/issue38316 Automerge-Triggered-By: @vstinner

@vstinner

(cherry picked from commit d587272) Co-authored-by: Batuhan Taşkaya <[email protected]> https://bugs.python.org/issue38316 Automerge-Triggered-By: @vstinner

the-knights-who-say-ni added the CLA signed label Oct 29, 2019

bedevere-bot added docs Documentation in the Doc dir awaiting review labels Oct 29, 2019

isidentical force-pushed the bpo-38316 branch from 92abd96 to 728a886 Compare October 29, 2019 12:35

ammaraskar added the skip news label Oct 29, 2019

ammaraskar reviewed Oct 29, 2019

View reviewed changes

Comment thread Doc/reference/datamodel.rst Outdated

isidentical force-pushed the bpo-38316 branch from 728a886 to 95c86b9 Compare October 30, 2019 15:18

isidentical requested a review from ammaraskar November 15, 2019 13:38

isidentical force-pushed the bpo-38316 branch from 95c86b9 to 09b25b1 Compare November 15, 2019 22:20

ammaraskar approved these changes Nov 15, 2019

View reviewed changes

Comment thread Doc/reference/datamodel.rst Outdated

bedevere-bot added awaiting core review and removed awaiting review labels Nov 15, 2019

vstinner requested changes Dec 14, 2019

View reviewed changes

bedevere-bot removed the awaiting core review label Dec 14, 2019

bedevere-bot added the awaiting changes label Dec 14, 2019

remove phrase

da1ab34

isidentical force-pushed the bpo-38316 branch from abbc1f1 to da1ab34 Compare December 15, 2019 06:41

bedevere-bot added awaiting change review and removed awaiting changes labels Dec 15, 2019

bedevere-bot requested a review from vstinner December 15, 2019 06:42

vstinner merged commit d587272 into python:master Dec 15, 2019

bedevere-bot removed the awaiting change review label Dec 15, 2019

vstinner added needs backport to 3.7 labels Dec 15, 2019

miss-islington assigned vstinner Dec 15, 2019

isidentical added a commit to isidentical/cpython that referenced this pull request Dec 19, 2019

[3.7] bpo-38316: Fix co_stacksize documentation (pythonGH-16983).

0973c24

(cherry picked from commit d587272) Co-authored-by: Batuhan Taşkaya <[email protected]>

bedevere-bot removed the needs backport to 3.7 label Dec 19, 2019

isidentical added a commit to isidentical/cpython that referenced this pull request Dec 19, 2019

[3.8] bpo-38316: Fix co_stacksize documentation (pythonGH-16983)

60e6279

(cherry picked from commit d587272) Co-authored-by: Batuhan Taşkaya <[email protected]>

bedevere-bot removed the needs backport to 3.8 label Dec 19, 2019

isidentical deleted the bpo-38316 branch December 19, 2019 14:51

shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020

bpo-38316: Fix co_stacksize documentation (pythonGH-16983)

d6e64b3

Conversation

isidentical commented Oct 29, 2019 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ammaraskar commented Nov 15, 2019

Uh oh!

isidentical commented Nov 15, 2019

Uh oh!

ammaraskar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ammaraskar commented Nov 15, 2019

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Dec 14, 2019

Uh oh!

isidentical commented Dec 15, 2019

Uh oh!

bedevere-bot commented Dec 15, 2019

Uh oh!

miss-islington commented Dec 15, 2019

Uh oh!

miss-islington commented Dec 15, 2019

Uh oh!

miss-islington commented Dec 15, 2019

Uh oh!

miss-islington commented Dec 15, 2019

Uh oh!

vstinner commented Dec 15, 2019

Uh oh!

vstinner commented Dec 18, 2019

Uh oh!

isidentical commented Dec 19, 2019

Uh oh!

bedevere-bot commented Dec 19, 2019

Uh oh!

bedevere-bot commented Dec 19, 2019

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

isidentical commented Oct 29, 2019 •

edited by bedevere-bot

Loading