NotificationsYou must be signed in to change notification settings
Fork32.4k
Star67.9k

gh-108113: Make it possible to optimize an AST#108282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

iritkatriel merged 2 commits intopython:mainfromiritkatriel:ast_parse2

Aug 23, 2023

Merged

gh-108113: Make it possible to optimize an AST#108282

iritkatriel merged 2 commits intopython:mainfromiritkatriel:ast_parse2

Aug 23, 2023

Conversation

Copy link

Member

iritkatriel commentedAug 22, 2023•
edited by bedevere-bot
Loading

Issue:compile() with PyCF_ONLY_AST flag ignores the optimize arg #108113

pythongh-108113: Make it possible to optimize and AST

b214797

iritkatriel requested a review frommarkshannon as acode owner

August 22, 2023 12:22

bedevere-bot mentioned this pull request

Aug 22, 2023

compile() with PyCF_ONLY_AST flag ignores the optimize arg#108113

Closed

bedevere-bot added the awaiting core review label

Aug 22, 2023

iritkatriel requested a review fromcarljm

August 22, 2023 12:23

iritkatriel added the skip news label

Aug 22, 2023

Copy link

MemberAuthor

iritkatriel commentedAug 22, 2023

Skipping news as the previous PR's news covers this.

carljm approved these changes

Aug 22, 2023

View reviewed changes

Python/bltinmodule.c Outdated

		@@ -804,25 +804,39 @@ builtin_compile_impl(PyObject module, PyObject source, PyObject *filename,
		if (is_ast == -1)
		goto error;
		if (is_ast) {
		PyArena *arena = _PyArena_New();

Copy link

Member

carljmAug 22, 2023

Choose a reason for hiding this comment

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

I wonder if anyone has perf-sensitive code that doesast.parse on an AST as a no-op, and what the impact on that code will be of allocating and freeing an unused arena. Probably not an issue? I think it would be possible, but slightly more complex, to still avoid arena allocation in the no-op case.