compiler: Unnecessary None in co_consts

[methane]

BPO	45056
Nosy	@gvanrossum, @methane, @ambv, @markshannon, @corona10, @pablogsal
PRs	bpo-45056: Remove trailing unused constants from co_consts #28109 [3.10] bpo-45056: Remove trailing unused constants from co_consts (GH-28109) #28125
Files	dump_unused_consts.py: script to dump unused constants unused_39.txt: Unused constants for _pyio at Python 3.9 unused_310rc1.txt: Unused constants for _pyio at Python 3.10rc1 unused_trimmed.txt: Unused constants for _pyio at What's new item for PEP 526 -- Variable annotations #72296

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2021-09-08.16:26:11.923>
created_at = <Date 2021-08-31.03:41:56.613>
labels = ['interpreter-core', '3.10', '3.11']
title = 'compiler: Unnecessary None in co_consts'
updated_at = <Date 2021-09-08.16:26:11.922>
user = 'https://github.com/methane'

bugs.python.org fields:

activity = <Date 2021-09-08.16:26:11.922>
actor = 'lukasz.langa'
assignee = 'none'
closed = True
closed_date = <Date 2021-09-08.16:26:11.923>
closer = 'lukasz.langa'
components = ['Interpreter Core']
creation = <Date 2021-08-31.03:41:56.613>
creator = 'methane'
dependencies = []
files = ['50252', '50253', '50254', '50255']
hgrepos = []
issue_num = 45056
keywords = ['patch', '3.10regression']
message_count = 13.0
messages = ['400683', '400687', '400688', '400818', '400823', '400824', '400915', '401007', '401017', '401021', '401023', '401397', '401398']
nosy_count = 6.0
nosy_names = ['gvanrossum', 'methane', 'lukasz.langa', 'Mark.Shannon', 'corona10', 'pablogsal']
pr_nums = ['28109', '28125']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue45056'
versions = ['Python 3.10', 'Python 3.11']

[methane]

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)

[gvanrossum]

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

[methane]

I think LOAD_CONST None + RETURN_VALUE is added here. And it removed by optimize_cfg().

cpython/Python/compile.c

Lines 7795 to 7797 in 793f55b

	if (addNone)
	ADDOP_LOAD_CONST(c, Py_None);
	ADDOP(c, RETURN_VALUE);

I don't know how easy to remove this unnecessary None.
But LOAD_COMMON_CONST idea can remove all None from co_consts.
iritkatriel#27

[methane]

I found anther example for unused constants.

The readall() function.

cpython/Lib/_pyio.py

Lines 663 to 675 in 863154c

	def readall(self):
	"""Read until EOF, using multiple read() call."""
	res = bytearray()
	while True:
	data = self.read(DEFAULT_BUFFER_SIZE)
	if not data:
	break
	res += data
	if res:
	return bytes(res)
	else:
	# b'' or None
	return data

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)

[methane]

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

#72296 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.

[pablogsal]

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.

[ambv]

New changeset 55c4a92 by Inada Naoki in branch 'main':
bpo-45056: Remove trailing unused constants from co_consts (GH-28109)
55c4a92

[pablogsal]

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.

[gvanrossum]

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.