Skip to content

gh-126835: Remove unused docstring const - #130016

Open
WolframAlph wants to merge 2 commits into
python:mainfrom
WolframAlph:unused-const
Open

gh-126835: Remove unused docstring const#130016
WolframAlph wants to merge 2 commits into
python:mainfrom
WolframAlph:unused-const

Conversation

@WolframAlph

@WolframAlphWolframAlph commented Feb 11, 2025

Copy link
Copy Markdown
Contributor

If no docstring present, extra unused constant from CFG optimization pass will reuse always preserved slot for docstring which leads to unused constant in co_consts. This breaks some unrelated tests when migrating optimizations from AST to CFG. Bug example:

@support.cpython_only
deftest_remove_unused_consts(self):
deff():
"docstring"
ifTrue:
return"used"
else:
return"unused"
self.assertEqual(f.__code__.co_consts,
(f.__doc__, "used"))
@support.cpython_only
deftest_remove_unused_consts_no_docstring(self):
# the first item (None for no docstring in this case) is
# always retained.
deff():
ifTrue:
return"used"
else:
return"unused"
self.assertEqual(f.__code__.co_consts,
(True, "used"))

First test case does not have True in co_consts but second one does even though it is not used (not used in first either) which is funny. Another example:

deff():
returnxprint(f.__code__.co_consts) # (None,)

None is in co_consts even though None is not used in f. It sneaks in by implicitly adding return None when compiling code unit, and it stays there because of always preserved slot for docstring.

Related comment: #126835 (comment)

@WolframAlphWolframAlph changed the title remove unused docstring constRemove unused docstring constFeb 11, 2025
@WolframAlphWolframAlph changed the title Remove unused docstring constgh-130016: Remove unused docstring constFeb 11, 2025
@WolframAlphWolframAlph changed the title gh-130016: Remove unused docstring constgh-126835: Remove unused docstring constFeb 11, 2025
@WolframAlph
WolframAlph marked this pull request as ready for review February 11, 2025 21:25
@WolframAlph

WolframAlph commented Feb 11, 2025

Copy link
Copy Markdown
ContributorAuthor

@iritkatriel let me know if you prefer having this linked to a separate issue. And also if we need to add some tests for this & news entry.

Comment threadPython/flowgraph.c
index_map[i] = -1;
}
// The first constant may be docstring; keep it always.
index_map[0] = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe update the comment above? It's only kept when we have the docstring now

@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actionsgithub-actionsBot added the stale Stale PR or inactive for long period of time. label Apr 21, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting reviewstaleStale PR or inactive for long period of time.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@WolframAlph@tomasr8@Eclips4