Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gh-101409: Improve generated clinic code for self type checks #101411

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

gh-101409: Improve generated clinic code for self type checks #101411

wants to merge 3 commits into from
+80 −78

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Jan 29, 2023 •
edited by bedevere-bot

Store the self type pointer in a local variable.

@erlend-aasland
pythongh-101409: Improve generated clinic code for self type checks
056abc4 
Store the self type pointer in a local variable.
@bedevere-bot bedevere-bot mentioned this pull request Jan 29, 2023
Open
@erlend-aasland
Copy link
Contributor Author

cc. @colorfulappl

@erlend-aasland erlend-aasland mentioned this pull request Jan 29, 2023
Open
@rhettinger rhettinger removed their request for review Jan 29, 2023
@colorfulappl
Copy link
Contributor

I'm not sure if this improvement is worth it, though; it may be churn for no benefit.

IMHO, this improvement is worth it.

  1. If the clinic_state() macro is fairly complicated, the compiler won't eliminate the redundant function invocations.

  2. The generated code after this patch is more concise and readable.

@erlend-aasland
Copy link
Contributor Author

  1. If the clinic_state() macro is fairly complicated, the compiler won't eliminate the redundant function invocations.

That is true; we cannot guarantee that the compiler will eliminate the redundant calls.

  1. The generated code after this patch is more concise and readable.

I agree with this.

Thanks for the review, @colorfulappl; highly appreciated.

serhiy-storchaka
serhiy-storchaka reviewed Jan 30, 2023
View changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

self_tp may be not a good name. It is not a type of self. Maybe base_type or declared_type or something?

@erlend-aasland
Copy link
Contributor Author

self_tp may be not a good name. It is not a type of self. Maybe base_type or declared_type or something?

base_type sounds good; thanks!

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

@ethanfurman ethanfurman

@berkerpeksag berkerpeksag

@serhiy-storchaka serhiy-storchaka

Assignees
No one assigned
Labels
awaiting core review skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@erlend-aasland @colorfulappl @serhiy-storchaka @bedevere-bot