-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
bpo-33216: Clarify the documentation for CALL_FUNCTION_* #8338
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
bpo-33216: Clarify the documentation for CALL_FUNCTION_* #8338
Conversation
Note that these bytecodes changed dramatically for 3.6. So 3.5 is the latest version where these documentation changes are relevant. If this PR is accepted I will probably attempt to backport these changes to 3.4. |
Doc/library/dis.rst
Outdated
Calls a function, similarly to :opcode:`CALL_FUNCTION`. | ||
*argc* represents the number of keyword and positional | ||
arguments, identically to :opcode:`CALL_FUNCTION`. | ||
The top of the stack contains an iterable object containing additional positional |
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.
An iterable object containing additional positional arguments was at the top of the stack before 3.5, but it is below keyword arguments in 3.5. This issue was opened for documenting this subtle change. Even after reading the code it can be missed. Please add also the versionchanged
directive.
The documentation for CALL_FUNCTION_VAR_KW
needs the same change.
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.
By golly, you're right! Gosh that's subtle. I've corrected the corrections ;-) and pushed an update to the PR.
It may be simpler to merge the current PR in 3.5, 3.4 and 2.7, and then create a separate PR which will document the 3.5 changes and fix bpo-33216. |
PR has been updated with the corrections pointed out by Serhiy. Thanks, Serhiy! |
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.
Do you mind to add versionchanged directives for highlighting the changes?
Doc/library/dis.rst
Outdated
@@ -1071,7 +1108,7 @@ instructions: | |||
|
|||
.. data:: hasconst | |||
|
|||
Sequence of bytecodes that have a constant parameter. | |||
Sequence of bytecodes that have a constant argument. |
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 think "Sequence of bytecodes that access a constant.
" would be more accurate (if it is a correct English). argc is an index in the list of constants (the co_consts
attribute of the code object).
Currently, in all maintained versions, hasconst
contains only LOAD_CONST
.
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. I only touched it because I was searching for "parameter", because nearly all the existing uses of the word "parameter" were wrong. But you're right, this one was completely wrong from the beginning. Will update the PR in a sec.
I'm +0 on adding "versionchanged" directives. I don't know what changed in what version, so you would have to tell me. But also, I don't know who would find the "versionchanged" information useful. Everything changed in 3.6, and 3.7 is out now; how many people in the universe need to know about new changes to bytecodes in 3.5? |
Before 3.5 an iterable object containing additional positional arguments was above keyword arguments (as you wrote in the initial edition). In 3.5 it is below keyword arguments. This is a side effect of implementing PEP 448. .. versionchanged:: 3.5
An iterable object containing additional positional arguments was above keyword arguments before Python 3.5. This information would be useful for authors of third-party projects which analyze or generate bytecode. |
Done. |
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.
Many thanks!
@larryhastings: Please replace |
Do you mind to backport this change to 2.7 (without 3.5 specific changes) and clarify the documentation in 3.6+? |
…) (pythonGH-8784) (cherry picked from commit 5e99b56) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
…) (pythonGH-8784) (cherry picked from commit 5e99b56) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
This is my take on #6365. I spent some time reading the code so I could understand exactly what the bytecodes do, then figured out how best to describe it.
I put off 3.5.6rc1 to work on this. I'm going to take a few hours off; when I come back I'm going to tag 3.5.6rc1. If I can get a quick review before I do that, I'll merge this in to 3.5 and it'll go into 3.5.6rc1.
https://bugs.python.org/issue33216