Skip to content

Conversation

@Zheaoli
Copy link
Contributor

@ZheaoliZheaoli commented Jan 4, 2026

Copy link
Member

@Fidget-SpinnerFidget-Spinner left a comment

Choose a reason for hiding this comment

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

Pretty close


op(_TO_BOOL_STR, (value -- res)){
op(_TO_BOOL_STR, (value -- res, v)){
int already_bool = optimize_to_bool(this_instr, ctx, value, &res);
Copy link
Member

Choose a reason for hiding this comment

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

This optimization is broken because the stack effect changed,

You need to fix optimize_to_bool to allow for an insert mode, so it should be the same as the one in her https://github.com/python/cpython/pull/143335/files (see _INSERT_1_LOAD_CONST_INLINE_BORROW)

So optimize_to_bool(this_instr, ctx, value, &res, true);, where true is for insert_mode, which will change to opcode to _INSERT_1_LOAD_CONST_INLINE_BORROW

Copy link
Member

Choose a reason for hiding this comment

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

I can do this change, as it's a little more complex

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Thanks for the suggestion! TIL

I may need a little bit time to update the patch(lol

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I can do this change, as it's a little more complex

If this is possible, I would like to make this change!

Copy link
Member

Choose a reason for hiding this comment

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

Sure!

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I have complete the code on my local branch, not push. Because I have some issue about this part

I think the patch, I will use _INSERT_1_LOAD_CONST_INLINE_BORROW but this is made in #143335

For now, I add a tier2 op named _INSERT_1_LOAD_CONST_INLINE_BORROW copy from #143335. I'm not sure if this is OK?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah just copy the op over

Signed-off-by: Manjusaka <[email protected]>
@Fidget-Spinner
Copy link
Member

Please fix the merge conflicts. Thanks!

Signed-off-by: Manjusaka <[email protected]>
@Zheaoli
Copy link
ContributorAuthor

Please fix the merge conflicts. Thanks!

Sorry about this, seems new commit merged in main. Fixed

@Fidget-SpinnerFidget-Spinner merged commit 0a5c04a into python:mainJan 6, 2026
70 checks passed
@ZheaoliZheaoli deleted the manjusaka/to-bool-str branch January 7, 2026 07:22
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Zheaoli@Fidget-Spinner