bpo-28810: Document remaining bytecode changes in 3.6 #651

ilevkivskyi · 2017-03-13T00:17:55Z

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

mention-bot · 2017-03-13T00:17:56Z

@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 · 2017-03-13T05:45:22Z

Doc/library/dis.rst

@@ -280,6 +287,7 @@ details of bytecode instructions as :class:`Instruction` instances:


 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 · 2017-03-13T05:51:18Z

Doc/library/dis.rst

@@ -210,6 +210,13 @@ operation is being performed, so the intermediate analysis object isn't useful:
   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 · 2017-03-13T08:27:59Z

@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 · 2017-03-13T10:17:37Z

Doc/library/dis.rst

@@ -210,6 +213,11 @@ operation is being performed, so the intermediate analysis object isn't useful:
   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 · 2017-03-13T11:23:42Z

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 · 2017-03-13T17:36:48Z

@ilevkivskyi separate PR would be great!

ilevkivskyi · 2017-03-13T19:01:03Z

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 · 2017-03-13T19:14:45Z

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 · 2017-03-13T19:16:17Z

Doc/library/dis.rst

@@ -20,6 +20,9 @@ interpreter.
   between versions of Python.  Use of this module should not be considered to
   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 · 2017-03-14T07:30:44Z

@serhiy-storchaka Thanks, I implemented your comments.

serhiy-storchaka

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

vstinner · 2017-03-14T16:31:58Z

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 · 2017-03-17T23:08:57Z

Doc/library/dis.rst

@@ -20,6 +20,10 @@ interpreter.
   between versions of Python.  Use of this module should not be considered to
   work across Python VMs or Python releases.

+   .. 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 · 2017-03-17T23:10:04Z

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 · 2017-03-18T02:27:22Z

@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)

		@@ -280,6 +287,7 @@ details of bytecode instructions as :class:`Instruction` instances:


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

Uh oh!

bpo-28810: Document remaining bytecode changes in 3.6 #651

bpo-28810: Document remaining bytecode changes in 3.6 #651

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!

Uh oh!

serhiy-storchaka Mar 14, 2017 •

edited

Loading