bpo-28810: Document remaining bytecode changes in 3.6 by ilevkivskyi · Pull Request #651 · python/cpython

ilevkivskyi · Mar 13, 2017

@serhiy-storchaka @brettcannon
These are two remaining changes to close http://bugs.python.org/issue28810
This needs a backport to 3.6

mention-bot · Mar 13, 2017

@ilevkivskyi, thanks for your PR! By analyzing the history of the files in this pull request, we identified @birkenfeld, @ncoghlan, @benjaminp, @serhiy-storchaka and @1st1 to be potential reviewers.

serhiy-storchaka · Mar 13, 2017

Doc/library/dis.rst



 The Python compiler currently generates the following bytecode instructions.
+Every instruction takes 2 bytes.


I think this should be moved to the top of the document, under impl-detail block, and decorated with versionchanged.

serhiy-storchaka · Mar 13, 2017

Doc/library/dis.rst

   This generator function uses the ``co_firstlineno`` and ``co_lnotab``
   attributes of the code object *code* to find the offsets which are starts of
   lines in the source code.  They are generated as ``(offset, lineno)`` pairs.
+   Functions decoding directly ``co_lnotab`` should use a signed


Too much details for common user of the dis module, but too few details for reproducing decoding algorithm. I think just adding the reference to Objects/lnotab_notes.txt is enough.

ilevkivskyi · Mar 13, 2017

@serhiy-storchaka Thanks!

I think this should be moved to the top of the document, under impl-detail block

I was also thinking about this. I implemented both your comments in a new commit.

ncoghlan · Mar 13, 2017

Doc/library/dis.rst

   This generator function uses the ``co_firstlineno`` and ``co_lnotab``
   attributes of the code object *code* to find the offsets which are starts of
   lines in the source code.  They are generated as ``(offset, lineno)`` pairs.
+   See ``Objects/lnotab_notes.txt`` for the ``co_lnotab`` format and how to


Using a source ref here turn this into a suitable hyperlink to GitHub:

:source:`Objects/lnotab_notes.txt`

Thank you, Nick!
Implemented this is a new commit.

ilevkivskyi · Mar 13, 2017

By the way, I have noticed that the example in Objects/lnotab_notes.txt is outdated. It still uses Python 2 and old bytecode (not 2 bytes per instruction).

Should I update it in this PR or open a separate issue and make a separate PR?

brettcannon · Mar 13, 2017

@ilevkivskyi separate PR would be great!

ilevkivskyi · Mar 13, 2017

separate PR would be great!

OK, will do this.
So it looks like this PR could be merged now.

serhiy-storchaka

The meaning of HAVE_ARGUMENT was changed too. Now all instructions have an argument, but opcodes < HAVE_ARGUMENT ignore it.

serhiy-storchaka · Mar 13, 2017

Doc/library/dis.rst

+   how to decode it.
+
+   .. versionchanged:: 3.6
+      Added support for negative line number delta.


This looks as implementation detail that should be described in lnotab_notes.txt. The change visible for the user of findlinestarts() is that line numbers now can not be monotonically increasing.

serhiy-storchaka · Mar 13, 2017

Doc/library/dis.rst

   work across Python VMs or Python releases.

+   .. versionchanged:: 3.6
+      Use fixed 2 bytes per instruction for all instructions.


May be worth to mention that instructions of variable length was used before.

…python into more-bytecode-docs

ilevkivskyi · Mar 14, 2017

@serhiy-storchaka Thanks, I implemented your comments.

serhiy-storchaka

I don't know whether the changes use good English, but in general LGTM.

vstinner · Mar 14, 2017

Doc/library/dis.rst

+   how to decode it.
+
+   .. versionchanged:: 3.6
+      Line numbers can be not monotonically increasing.


"not monotonically" is not easy to understand. In What's New in Python 3.6, I wrote:

"The format of the co_lnotab attribute of code objects changed to support a negative line number delta"

This is how I initially wrote it, but @serhiy-storchaka wanted me to change this to this wording.
His motivation is that negative delta is not a visible effect for user, see #651 (comment)

This is the wording for describing the implementation details of co_lnotab. What is important for the user of findlinestarts() which shouldn't know anything about co_lnotab is that now this function can produce non-monotonic line numbers. That can affect user code.

Just say "Line numbers can now go backwards." if you prefer. I would like to avoid "monotonically", especially "not monotonically".

Line numbers can now go backwards.

This sounds not very clear to me. I changed this to an alternative formulation "Line numbers can be decreasing. Before, they were always increasing." I hope this works.

brettcannon · Mar 17, 2017

Doc/library/dis.rst


+   .. versionchanged:: 3.6
+      Use fixed 2 bytes per instruction for all instructions.  Instructions
+      of variable length were used before.


I would phrase this as "Use 2 bytes for each instruction. Previously the number of bytes varied by instruction."

brettcannon · Mar 17, 2017

Doc/library/dis.rst

+   those which do ``>= HAVE_ARGUMENT``.
+
+   .. versionchanged:: 3.6
+      Now every instruction have an argument, but opcodes ``< HAVE_ARGUMENT``


"Now every instruction has an argument"

ilevkivskyi · Mar 18, 2017

@brettcannon Thanks! I fixed both points in a new commit.

(cherry picked from commit 8f9e1bb)

Document remaining bytecode changes in 3.6

51e7616

the-knights-who-say-ni added the CLA signed label Mar 13, 2017

serhiy-storchaka reviewed Mar 13, 2017

View reviewed changes

serhiy-storchaka added type-feature A feature request or enhancement needs backport to 3.6 docs Documentation in the Doc dir and removed type-feature A feature request or enhancement labels Mar 13, 2017

Response to Serhiy's review

1b1034d

ncoghlan reviewed Mar 13, 2017

View reviewed changes

Add link to source

d726673

serhiy-storchaka reviewed Mar 13, 2017

View reviewed changes

ilevkivskyi added 2 commits March 14, 2017 08:27

Response to comments

f1e17fc

Merge branch 'more-bytecode-docs' of https://github.com/ilevkivskyi/c…

bcd7912

…python into more-bytecode-docs

serhiy-storchaka approved these changes Mar 14, 2017

View reviewed changes

vstinner reviewed Mar 14, 2017

View reviewed changes

Avoid 'non monotonically'

4ebbc2a

brettcannon requested changes Mar 17, 2017

View reviewed changes

Response to comments

400b114

brettcannon self-assigned this Mar 20, 2017

Make a sentence easier to read

b1034c0

brettcannon approved these changes Mar 24, 2017

View reviewed changes

brettcannon merged commit 8f9e1bb into python:master Mar 24, 2017

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

bpo-28810: Document remaining bytecode changes in 3.6 (pythonGH-651)

81230f9

(cherry picked from commit 8f9e1bb)

brettcannon removed the needs backport to 3.6 label Mar 24, 2017

brettcannon added a commit that referenced this pull request Mar 24, 2017

bpo-28810: Document remaining bytecode changes in 3.6 (GH-651) (GH-808)

74bfcc3

(cherry picked from commit 8f9e1bb)



		The Python compiler currently generates the following bytecode instructions.
		Every instruction takes 2 bytes.

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

Uh oh!

Conversation

ilevkivskyi commented Mar 13, 2017

Uh oh!

mention-bot commented Mar 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ilevkivskyi commented Mar 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ilevkivskyi commented Mar 13, 2017

Uh oh!

brettcannon commented Mar 13, 2017

Uh oh!

ilevkivskyi commented Mar 13, 2017

Uh oh!

serhiy-storchaka left a comment

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 Mar 14, 2017

Uh oh!

serhiy-storchaka left a comment

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!

serhiy-storchaka Mar 14, 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!

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 Mar 18, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

serhiy-storchaka Mar 14, 2017 •

edited

Loading