classification
Title: Opcode names BUILD_MAP_UNPACK_WITH_CALL and BUILD_TUPLE_UNPACK_WITH_CALL are too long
Type: enhancement Stage: patch review
Components: Documentation Versions: Python 3.9
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: docs@python Nosy List: Jim Fasarakis-Hilliard, ZackerySpytz, docs@python, ncoghlan, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2017-04-15 17:27 by serhiy.storchaka, last changed 2019-09-11 22:54 by ncoghlan.

Pull Requests
URL Status Linked Edit
PR 15513 open ZackerySpytz, 2019-08-26 10:24
Messages (8)
msg291725 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-15 17:27
Two new opcodes BUILD_MAP_UNPACK_WITH_CALL and BUILD_TUPLE_UNPACK_WITH_CALL (added in 3.5 and 3.6) have too long names (26 and 28 characters). This exceeds the width of the opcode names in the dis module (20 characters). Increasing the width of the column will make opcode arguments too distant from opcode names, that will decrease readability. The better solution would be renaming these two opcodes.

They are used for merging iterables and mappings of positional and keyword arguments when used var-positional (*args) and var-keyword (**kwargs) arguments. Maybe new names should reflect this.

>>> dis.dis('f(a, b, *args, x=x, y=y, **kw)')
  1           0 LOAD_NAME                0 (f)
              2 LOAD_NAME                1 (a)
              4 LOAD_NAME                2 (b)
              6 BUILD_TUPLE              2
              8 LOAD_NAME                3 (args)
             10 BUILD_TUPLE_UNPACK_WITH_CALL     2
             12 LOAD_NAME                4 (x)
             14 LOAD_NAME                5 (y)
             16 LOAD_CONST               0 (('x', 'y'))
             18 BUILD_CONST_KEY_MAP      2
             20 LOAD_NAME                6 (kw)
             22 BUILD_MAP_UNPACK_WITH_CALL     2
             24 CALL_FUNCTION_EX         1
             26 RETURN_VALUE
msg291778 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-04-17 04:32
If we used the names from inspect.Parameter.kind (https://docs.python.org/3/library/inspect.html#inspect.Parameter.kind) these would become:

* BUILD_VAR_POSITIONAL
* BUILD_VAR_KEYWORD

which would be 20 and 17 letters long, respectively.

I think that would also make the disassembled bytecode more self-explanatory:

>>> dis.dis('f(a, b, *args, x=x, y=y, **kw)')
  1           0 LOAD_NAME                0 (f)
              2 LOAD_NAME                1 (a)
              4 LOAD_NAME                2 (b)
              6 BUILD_TUPLE              2
              8 LOAD_NAME                3 (args)
             10 BUILD_VAR_POSITIONAL     2
             12 LOAD_NAME                4 (x)
             14 LOAD_NAME                5 (y)
             16 LOAD_CONST               0 (('x', 'y'))
             18 BUILD_CONST_KEY_MAP      2
             20 LOAD_NAME                6 (kw)
             22 BUILD_VAR_KEYWORD        2
             24 CALL_FUNCTION_EX         1
             26 RETURN_VALUE
msg291792 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-17 09:15
I like the names BUILD_VAR_POSITIONAL and BUILD_VAR_KEYWORD.

The downside is that this breaks relations between them and very similar opcodes BUILD_TUPLE_UNPACK and BUILD_MAP_UNPACK. The only differences between BUILD_*_UNPACK and BUILD_*_UNPACK_WITH_CALL opcodes is that the latter takes a function name from the stack for error message and BUILD_MAP_UNPACK_WITH_CALL doesn't allow key duplications in contrary to BUILD_MAP_UNPACK. But the names BUILD_TUPLE_UNPACK and BUILD_MAP_UNPACK doesn't look very good too.
msg291832 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-04-18 12:35
I think it should be sufficient connection to describe the link in the documentation as:

* BUILD_VAR_POSITIONAL: a variant of BUILD_TUPLE_UNPACK tailored specifically for use in function calls
* BUILD_VAR_KEYWORD: a variant of BUILD_MAP_UNPACK tailored specifically for use in function calls

The BUILD_*_UNPACK documentation could also point to their relevant function call counterpart: "Note: function calls do not use this opcode, they use the specialized BUILD_VAR_<kind>".
msg350519 - (view) Author: Zackery Spytz (ZackerySpytz) * (Python triager) Date: 2019-08-26 10:26
I have created a pull request for this issue. Please take a look.
msg351080 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-09-03 10:59
Reviewing this again now, I think my previous naming suggestion is problematic, as it encourages conflating two different concepts that use similar syntax:

* collecting arbitrary positional parameters in a tuple (VAR_POSITIONAL) or arbitrary keyword parameters in a dictionary (VAR_POSITIONAL, VAR_KEYWORD)
* unpacking function arguments from iterables (BUILD_VAR_POSITIONAL) or mappings (BUILD_VAR_KEYWORD)

I think the fix for that error is straightforward though: replace "VAR" with "ARG" in the new opcode names, giving:

* BUILD_ARG_POSITIONAL
* BUILD_ARG_KEYWORD

That should also read nicely with Zackery's documentation updates.
msg351081 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-09-03 11:13
These opcodes are always used with CALL_FUNCTION_EX (which can be used also without any of them).

f(*a, *b, **c, **d)
  1           0 LOAD_NAME                0 (f)
              2 LOAD_NAME                1 (a)
              4 LOAD_NAME                2 (b)
              6 BUILD_TUPLE_UNPACK_WITH_CALL     2
              8 LOAD_NAME                3 (c)
             10 LOAD_NAME                4 (d)
             12 BUILD_MAP_UNPACK_WITH_CALL     2
             14 CALL_FUNCTION_EX         1

I though about naming CALL_FUNCTION_EX as CALL_FUNCTION_VAR in 3.6, but this name was used in pre-3.6 bytecode with different semantic. Now I think there is smaller change of confusion, so it can be renamed to CALL_FUNCTION_VAR or something other.
msg352029 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-09-11 22:54
So we'd be going with a naming convention where "VAR" corresponds to the star syntax, and whether it means packing or unpacking, or refers to arguments or parameters is context dependent?

I could definitely support that, given that the ambiguity already exists at the Python syntax level.
History
Date User Action Args
2019-09-11 22:54:39ncoghlansetmessages: + msg352029
2019-09-03 11:13:43serhiy.storchakasetmessages: + msg351081
2019-09-03 10:59:56ncoghlansetmessages: + msg351080
2019-08-26 12:17:39serhiy.storchakasetversions: + Python 3.9, - Python 3.6, Python 3.7, Python 3.8
2019-08-26 10:26:10ZackerySpytzsetnosy: + ZackerySpytz
messages: + msg350519
2019-08-26 10:24:17ZackerySpytzsetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request15199
2018-03-22 10:21:05serhiy.storchakasetversions: + Python 3.6, Python 3.8
nosy: + docs@python

assignee: docs@python
components: + Documentation
stage: needs patch
2017-04-18 12:35:37ncoghlansetmessages: + msg291832
2017-04-17 09:15:31serhiy.storchakasetmessages: + msg291792
2017-04-17 08:44:07Jim Fasarakis-Hilliardsetnosy: + Jim Fasarakis-Hilliard
2017-04-17 04:32:28ncoghlansetmessages: + msg291778
2017-04-15 17:27:19serhiy.storchakacreate