Skip to content

Conversation

@Fidget-Spinner
Copy link
Member

@Fidget-Spinner Fidget-Spinner commented Jan 31, 2026

So the previous PR 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 the DEOPT_IF(attr != NULL) completely
  2. There's a bug with dealing with escapes. I added a test for that.

@Fidget-Spinner
Copy link
Member Author

@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 the last_escape_index here to prevent subsequent set_attr calls from repeatedly clearing num_descrs?

*(ctx->out_buffer.next++) = *this_instr;
}
// Track escapes - but skip when from init shim frame, since self hasn't escaped yet
bool is_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 the is_init_shim now.

@cocolato
Copy link
Contributor

cocolato commented Feb 1, 2026

Sorry for any undiscovered bugs, thanks for the fix!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants