bpo-26213: Document _UNPACK bytecodes and BUILD_MAP changes by ilevkivskyi · Pull Request #238 · python/cpython

ilevkivskyi · Feb 22, 2017

This needs to be backported to 3.5 and 3.6

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

serhiy-storchaka · Feb 22, 2017

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 · Feb 22, 2017

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


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

serhiy-storchaka · Feb 22, 2017

Doc/library/dis.rst

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


Mention what is changed.

serhiy-storchaka · Feb 22, 2017

Doc/library/dis.rst

   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 · Feb 22, 2017

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 · Feb 22, 2017

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

serhiy-storchaka · Feb 22, 2017

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 · Feb 22, 2017

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 · Feb 22, 2017

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 · Feb 22, 2017

Thanks! I have implemented new comments in another commit.

ilevkivskyi · Feb 22, 2017

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 · Feb 23, 2017

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 · Feb 23, 2017

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 · Feb 24, 2017

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 · Feb 24, 2017

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 · Mar 3, 2017

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 · Mar 4, 2017

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

		.. versionadded:: 3.5


		.. opcode:: BUILD_MAP_UNPACK_WITH_CALL (count)

		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,

Search code, repositories, users, issues, pull requests...

Uh oh!

Conversation

ilevkivskyi commented Feb 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ilevkivskyi commented Feb 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ilevkivskyi commented Feb 22, 2017

Uh oh!

ilevkivskyi commented Feb 22, 2017

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

ilevkivskyi commented Feb 23, 2017

Uh oh!

ilevkivskyi commented Feb 23, 2017

Uh oh!

DimitrisJim Feb 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ilevkivskyi commented Feb 24, 2017

Uh oh!

brettcannon commented Mar 3, 2017

Uh oh!

serhiy-storchaka commented Mar 4, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

DimitrisJim Feb 24, 2017 •

edited

Loading