This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: compiler: Unnecessary None in co_consts
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.11, Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Mark.Shannon, corona10, gvanrossum, lukasz.langa, methane, pablogsal
Priority: normal Keywords: 3.10regression, patch

Created on 2021-08-31 03:41 by methane, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
dump_unused_consts.py methane, 2021-09-02 05:29 script to dump unused constants
unused_39.txt methane, 2021-09-02 05:30 Unused constants for _pyio at Python 3.9
unused_310rc1.txt methane, 2021-09-02 05:30 Unused constants for _pyio at Python 3.10rc1
unused_trimmed.txt methane, 2021-09-02 05:30 Unused constants for _pyio at GH-28109
Pull Requests
URL Status Linked Edit
PR 28109 merged methane, 2021-09-01 08:16
PR 28125 merged lukasz.langa, 2021-09-02 14:49
Messages (13)
msg400683 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2021-08-31 03:41
Python 3.10 compiler adds None to co_consts even when None is not used at all.

```
$ cat x1.py
def foo():
    "docstring"
    return 42

import dis
dis.dis(foo)
print(foo.__code__.co_consts)

$ python3.9 x1.py
  3           0 LOAD_CONST               1 (42)
              2 RETURN_VALUE
('docstring', 42)

$ python3.10 x1.py
  3           0 LOAD_CONST               1 (42)
              2 RETURN_VALUE
('docstring', 42, None)
```
msg400687 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-08-31 04:24
I wonder what the 3.10 compiler does different to cause this -- in 3.9 that extra None is not in co_consts.

(Note: Mark is on vacation for 2 weeks.)
msg400688 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2021-08-31 04:39
I think LOAD_CONST None + RETURN_VALUE is added here. And it removed by optimize_cfg().

https://github.com/python/cpython/blob/793f55bde9b0299100c12ddb0e6949c6eb4d85e5/Python/compile.c#L7795-L7797

I don't know how easy to remove this unnecessary None.
But LOAD_COMMON_CONST idea can remove all None from co_consts.
https://github.com/iritkatriel/cpython/pull/27
msg400818 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2021-09-01 07:22
I found anther example for unused constants.

The `readall()` function.
https://github.com/python/cpython/blob/863154c9292e70c5a8a1a3f22ef4ee42e2304281/Lib/_pyio.py#L663-L675

Python 3.9.5:
co_consts = ('Read until EOF, using multiple read() call.', None)
Python 3.10rc1:
co_consts = ('Read until EOF, using multiple read() call.', True, None)
msg400823 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2021-09-01 08:22
@Pablo This issue seems an optimization rather than bugfix.
But since Python 3.10 produces more unused constants than 3.9, this is a regression.

GH-28109 removes not all unused constants, but only trailing unused constants.
I think it is enough to fix the regression.

I want to backport it to 3.10 or 3.10.1 after Mark's review.
msg400824 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-09-01 09:08
> I want to backport it to 3.10 or 3.10.1 after Mark's review.

Ok, please, add me as a reviewer to the backport once is ready.
msg400915 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-09-02 11:02
New changeset 55c4a92fc1abfe388335071f1d64b3addfa5793f by Inada Naoki in branch 'main':
bpo-45056: Remove trailing unused constants from co_consts (GH-28109)
https://github.com/python/cpython/commit/55c4a92fc1abfe388335071f1d64b3addfa5793f
msg401007 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-09-03 17:16
Let's wait for 3.10.1 to backport this because I prefer to keep pyc files stable for the release candidate. Turns out that people are already preparing wheels to 3.10 and although this may be fine, I don't want to risk incompatibilities for the release.
msg401017 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-09-03 20:12
Are you sure? I know we're really close to the release, but I'd much rather
change the PYC format during the RC cycle than once we're doing bugfix
releases. I don't think we've ever changed the magic number between bugfix
releases. (I realize this doesn't change the magic number, but it's
probably also the first time we make a change that affects marshal output
without requiring changes in marshal input.
msg401021 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-09-04 00:29
I honestly want to backport it because I think it should not have any negative impact, but on the other hand I don't feel very confident as the release candidate phase is supposed to be as close as possible as the final release and this is not fixing a critical bug. The devguide says about the RC:


>>Generally, these issues must be severe enough (e.g. crashes) that they deserve fixing before the final release. All other issues should be deferred to the next development cycle, since stability is the strongest concern at this point.

I am just trying to be cautious but on the other hand we still have anltbet release candidate for people to try it out before the final release so if you all think is better to have this on the RC and this is not going to be an issue for existing universal wheels and the like, then I suppose we can merge it before Rc2
msg401023 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-09-04 00:35
Since we're not changing the magic number, wheels created for rc1 will still work with the final 3.10 (and vice versa!) -- creating a wheel or PYC file just will produce a different sequence of bytes, but there are other things that do that.

Then again, since we're not changing the magic number, it's not the end of the world to put off the backport to 3.10.1.

So I've convinced myself that it actually doesn't matter, and probably it's best to wait for 3.10.1 (changes to the compiler are always risky).
msg401397 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-09-08 16:25
New changeset d41abe8970453716dbc6a3a898ac8fb01cbf6c6f by Łukasz Langa in branch '3.10':
[3.10] bpo-45056: Remove trailing unused constants from co_consts (GH-28109) (GH-28125)
https://github.com/python/cpython/commit/d41abe8970453716dbc6a3a898ac8fb01cbf6c6f
msg401398 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-09-08 16:26
It goes to 3.10.1 then. Fixed!
History
Date User Action Args
2022-04-11 14:59:49adminsetgithub: 89219
2021-09-08 16:26:11lukasz.langasetstatus: open -> closed
resolution: fixed
messages: + msg401398

stage: patch review -> resolved
2021-09-08 16:25:17lukasz.langasetmessages: + msg401397
2021-09-04 00:35:37gvanrossumsetmessages: + msg401023
2021-09-04 00:29:53pablogsalsetmessages: + msg401021
2021-09-03 20:12:53gvanrossumsetmessages: + msg401017
2021-09-03 17:16:21pablogsalsetmessages: + msg401007
2021-09-02 14:49:48lukasz.langasetpull_requests: + pull_request26564
2021-09-02 11:02:10lukasz.langasetnosy: + lukasz.langa
messages: + msg400915
2021-09-02 05:30:50methanesetfiles: + unused_trimmed.txt
2021-09-02 05:30:28methanesetfiles: + unused_310rc1.txt
2021-09-02 05:30:08methanesetfiles: + unused_39.txt
2021-09-02 05:29:48methanesetfiles: + dump_unused_consts.py
2021-09-01 09:08:30pablogsalsetmessages: + msg400824
2021-09-01 08:22:44methanesetnosy: + pablogsal

messages: + msg400823
versions: + Python 3.11
2021-09-01 08:16:35methanesetkeywords: + patch
stage: patch review
pull_requests: + pull_request26551
2021-09-01 07:22:39methanesetmessages: + msg400818
2021-08-31 05:57:40corona10setnosy: + corona10
2021-08-31 04:39:03methanesetmessages: + msg400688
2021-08-31 04:24:54gvanrossumsetnosy: + Mark.Shannon, gvanrossum
messages: + msg400687
2021-08-31 03:41:56methanecreate