Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-144145: Cleanups for object property tracking in JIT optimizer#144366

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

Open
Fidget-Spinner wants to merge3 commits intopython:main
base:main
Choose a base branch
Loading
fromFidget-Spinner:gh-144145-fixups

Conversation

@Fidget-Spinner
Copy link
Member

@Fidget-SpinnerFidget-Spinner commentedJan 31, 2026
edited
Loading

So the previousPR set a good foundation and was always right as it depends on a runtime check. However, I just noticed some bugs that need cleanup:

  1. we can do better and eliminate theDEOPT_IF(attr != NULL) completely
  2. There's a bug with dealing with escapes. I added a test for that.

@Fidget-Spinner
Copy link
MemberAuthor

@cocolato can you review this please?

// Check escape
if (sym->descr.last_escape_index < ctx->last_escape_index) {
sym->descr.num_descrs = 0;
return _Py_uop_sym_new_unknown(ctx);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return_Py_uop_sym_new_unknown(ctx);
sym->descr.num_descrs=0;
sym->descr.last_escape_index=uop_buffer_length(&ctx->out_buffer);

Can we update thelast_escape_index here to prevent subsequentset_attr calls from repeatedly clearingnum_descrs?

*(ctx->out_buffer.next++)=*this_instr;
}
// Track escapes - but skip when from init shim frame, since self hasn't escaped yet
boolis_init_shim=CURRENT_FRAME_IS_INIT_SHIM();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bool is_init_shim = CURRENT_FRAME_IS_INIT_SHIM();

Perhaps we can remove theis_init_shim now.

@cocolato
Copy link
Contributor

Sorry for any undiscovered bugs, thanks for the fix!

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@markshannonmarkshannonAwaiting requested review from markshannonmarkshannon is a code owner

@tomasr8tomasr8Awaiting requested review from tomasr8tomasr8 is a code owner

@savannahostrowskisavannahostrowskiAwaiting requested review from savannahostrowskisavannahostrowski is a code owner

1 more reviewer

@cocolatococolatococolato left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Fidget-Spinner@cocolato

[8]ページ先頭

©2009-2026 Movatter.jp