bpo-26213: Document _UNPACK bytecodes and BUILD_MAP changes #238

ilevkivskyi · 2017-02-22T13:32:52Z

This needs to be backported to 3.5 and 3.6

This fixes http://bugs.python.org/issue26213

serhiy-storchaka · 2017-02-22T13:50:49Z

Doc/library/dis.rst

+   .. versionadded:: 3.5
+
+
+.. opcode:: BUILD_MAP_UNPACK_WITH_CALL (count)


The semantic is different in 3.5. The lowest byte of oparg is the count of mappings, the relative position of the callable is encoded in the second byte of oparg. Thus needed ".. versionadded:: 3.6".

Or maybe first write a patch that describes 3.5 semantic and change it later in 3.6+ patch.

I will add the 3.6 semantics in a separate PR. In the new commit I decided to describe the 3.5 semantics.

serhiy-storchaka · 2017-02-22T13:53:54Z

Doc/library/dis.rst

@@ -772,8 +813,11 @@ All of the following opcodes use their arguments.

 .. opcode:: BUILD_MAP (count)

-   Pushes a new dictionary object onto the stack.  The dictionary is pre-sized
-   to hold *count* entries.
+   Pushes a new dictionary object onto the stack. Pops ``2 * count`` items


This is just a nitpick, but different PEPs recommend to use a double space after sentence ending period.

serhiy-storchaka · 2017-02-22T13:54:47Z

Doc/library/dis.rst

+   so that the dictionary holds ``count`` entries:
+   ``{..., TOS3: TOS2, TOS1: TOS}``.
+
+   .. versionchanged:: 3.5


Mention what is changed.

serhiy-storchaka · 2017-02-22T14:02:14Z

Doc/library/dis.rst

@@ -723,6 +723,47 @@ All of the following opcodes use their arguments.
   are put onto the stack right-to-left.


+.. opcode:: BUILD_TUPLE_UNPACK (count)


Following opcodes create the collection by concatenating/merging several values from the stack. I think it would be better to place their descriptions after BUILD_STRING (which is similar to BUILD_*_UNPACK opcodes but for strings).

serhiy-storchaka · 2017-02-22T14:12:03Z

Doc/library/dis.rst

+.. opcode:: BUILD_TUPLE_UNPACK (count)
+
+   Pops ``count`` iterables from the stack, joins them in a single tuple,
+   and pushes the result.


It may be worth to mention that it is used for implementing iterable unpacking in tuple displays (see https://docs.python.org/3/whatsnew/3.5.html#pep-465-a-dedicated-infix-operator-for-matrix-multiplication). Example: (*x, *y, *z).

ilevkivskyi · 2017-02-22T14:44:57Z

Serhiy, thank you for review! I implemented your comments in a new commit.

serhiy-storchaka · 2017-02-22T15:08:47Z

Doc/library/dis.rst

+
+   Pops ``count`` iterables from the stack, joins them in a single tuple,
+   and pushes the result.  This bytecode is used for implementing iterable
+   unpacking in tuple displays ``(*x, *y, *z)``.


Maybe add examples for other opcodes?

[*x, *y, *z]
{*x, *y, *z}
{**x, **y, **z}

serhiy-storchaka · 2017-02-22T15:11:47Z

Doc/library/dis.rst

-   Pushes a new dictionary object onto the stack.  The dictionary is pre-sized
-   to hold *count* entries.
+   Pushes a new dictionary object onto the stack.  Pops ``2 * count`` items
+   so that the dictionary holds ``count`` entries:


BUILD_TUPLE and others format the parameter as *count* rather than ``count``.

serhiy-storchaka · 2017-02-22T15:20:22Z

Doc/library/dis.rst

+
+.. opcode:: BUILD_MAP_UNPACK (count)
+
+   Pops ``count`` mappings from the stack, joins them in a single dictionary,


Maybe "merges" is more appropriate term than "joins".

ilevkivskyi · 2017-02-22T15:45:18Z

Thanks! I have implemented new comments in another commit.

ilevkivskyi · 2017-02-22T15:49:13Z

2 * count is an expression, all expressions are formatted with ``.

Fixed.

serhiy-storchaka

Now it LGTM. Thank you Ivan. But would be nice if some native speaker take a look.

ilevkivskyi · 2017-02-23T13:01:36Z

Now it LGTM. Thank you Ivan. But would be nice if some native speaker take a look.

@briancurtin Could you please also take a look at this?

Note that I will update the BUILD_MAP_UNPACK_WITH_CALL docs in a separate PR (or maybe I will just update #239) after this is merged.

ilevkivskyi · 2017-02-23T13:04:08Z

Now it LGTM. Thank you Ivan. But would be nice if some native speaker take a look.

@brettcannon Could you please also take a look at this?

Note that I will update the BUILD_MAP_UNPACK_WITH_CALL docs in a separate PR (or maybe I will just update #239) after this is merged.

@briancurtin Sorry for disturbing! This was intended for @brettcannon

DimitrisJim · 2017-02-24T20:00:41Z

Doc/library/dis.rst

+.. opcode:: BUILD_TUPLE_UNPACK (count)
+
+   Pops *count* iterables from the stack, joins them in a single tuple,
+   and pushes the result.  This bytecode is used for implementing iterable


Though not technically incorrect, using the sentence "This bytecode is used .." for every case is somewhat repetitive IMO.It might be better to use the same format the other opcodes in the docs use and go for:

Implements iterable unpacking for displays <type_display>

Other than that change, which I believe would be beneficial, the rest look good to me.

ilevkivskyi · 2017-02-24T22:37:58Z

Implements iterable unpacking for displays <type_display>
Other than that change, which I believe would be beneficial, the rest look good to me

Thanks! Fixed this in a new commit.

brettcannon · 2017-03-03T21:28:03Z

I tweaked a single word in the PR. Once Travis is green again I'll merge and backport.

(cherry picked from commit 0705f66)

serhiy-storchaka · 2017-03-04T06:28:16Z

Thank you Ivan and Brett.

Document _UNPACK bytecodes added in 3.5 and BUILD_MAP changes

73295ed

the-knights-who-say-ni added the CLA signed label Feb 22, 2017

zhangyangyu added docs Documentation in the Doc dir needs backport to 3.5 labels Feb 22, 2017

zhangyangyu requested a review from serhiy-storchaka February 22, 2017 13:47

ilevkivskyi mentioned this pull request Feb 22, 2017

bpo-28810: Document BUILD_TUPLE_UNPACK_WITH_CALL bytecode added in 3.6 #239

Merged

serhiy-storchaka requested changes Feb 22, 2017

View reviewed changes

Response to comments

5fe6291

Formatting (two spaces after period)

a53672b

serhiy-storchaka reviewed Feb 22, 2017

View reviewed changes

Response to comments, second round

9be8a6b

Fix formatting

5177cec

Fix terminology

8d5281d

serhiy-storchaka approved these changes Feb 22, 2017

View reviewed changes

serhiy-storchaka requested a review from brettcannon February 23, 2017 13:27

DimitrisJim suggested changes Feb 24, 2017

View reviewed changes

Response to comments (shorter formulation)

d32b1d9

DimitrisJim approved these changes Feb 24, 2017

View reviewed changes

Tweak a word

fb7507c

brettcannon approved these changes Mar 3, 2017

View reviewed changes

brettcannon changed the title ~~bpo-26213: Document _UNPACK bytecodes added in 3.5 and BUILD_MAP changes~~ bpo-26213: Document _UNPACK bytecodes and BUILD_MAP changes Mar 3, 2017

brettcannon merged commit 0705f66 into python:master Mar 3, 2017

brettcannon pushed a commit to brettcannon/cpython that referenced this pull request Mar 3, 2017

bpo-26213: Document _UNPACK bytecodes and BUILD_MAP changes (python#238)

c91a725

(cherry picked from commit 0705f66)

brettcannon pushed a commit to brettcannon/cpython that referenced this pull request Mar 3, 2017

bpo-26213: Document _UNPACK bytecodes and BUILD_MAP changes (python#238)

9fe1329

(cherry picked from commit 0705f66)

Mariatta removed needs backport to 3.5 labels Mar 6, 2017

jaraco pushed a commit that referenced this pull request Dec 2, 2022

Move coverage into dev-requirements.txt (#238)

9a30fdf

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-26213: Document _UNPACK bytecodes and BUILD_MAP changes #238

bpo-26213: Document _UNPACK bytecodes and BUILD_MAP changes #238

ilevkivskyi commented Feb 22, 2017

serhiy-storchaka Feb 22, 2017

ilevkivskyi Feb 22, 2017

serhiy-storchaka Feb 22, 2017

serhiy-storchaka Feb 22, 2017

serhiy-storchaka Feb 22, 2017

serhiy-storchaka Feb 22, 2017

ilevkivskyi commented Feb 22, 2017

serhiy-storchaka Feb 22, 2017

serhiy-storchaka Feb 22, 2017

serhiy-storchaka Feb 22, 2017

ilevkivskyi commented Feb 22, 2017

ilevkivskyi commented Feb 22, 2017

serhiy-storchaka left a comment

ilevkivskyi commented Feb 23, 2017

ilevkivskyi commented Feb 23, 2017

DimitrisJim Feb 24, 2017 •

edited

Loading

ilevkivskyi commented Feb 24, 2017

brettcannon commented Mar 3, 2017

serhiy-storchaka commented Mar 4, 2017

		.. versionadded:: 3.5


		.. opcode:: BUILD_MAP_UNPACK_WITH_CALL (count)

		@@ -723,6 +723,47 @@ All of the following opcodes use their arguments.
		are put onto the stack right-to-left.


		.. opcode:: BUILD_TUPLE_UNPACK (count)


		.. opcode:: BUILD_MAP_UNPACK (count)

		Pops ``count`` mappings from the stack, joins them in a single dictionary,

bpo-26213: Document _UNPACK bytecodes and BUILD_MAP changes #238

bpo-26213: Document _UNPACK bytecodes and BUILD_MAP changes #238

Conversation

ilevkivskyi commented Feb 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilevkivskyi commented Feb 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilevkivskyi commented Feb 22, 2017

ilevkivskyi commented Feb 22, 2017

serhiy-storchaka left a comment

Choose a reason for hiding this comment

ilevkivskyi commented Feb 23, 2017

ilevkivskyi commented Feb 23, 2017

DimitrisJim Feb 24, 2017 • edited Loading

Choose a reason for hiding this comment

ilevkivskyi commented Feb 24, 2017

brettcannon commented Mar 3, 2017

serhiy-storchaka commented Mar 4, 2017

DimitrisJim Feb 24, 2017 •

edited

Loading