-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
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! |
serhiy-storchaka
left a comment
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
| .. 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. |
serhiy-storchaka
left a comment
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