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

gh-99770: Make the correct call specialization fail kind show up #99771

Merged
merged 3 commits into from Dec 22, 2022

Conversation

penguin-wwy
Copy link
Contributor

@penguin-wwy penguin-wwy commented Nov 25, 2022

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Nov 25, 2022

All commit authors signed the Contributor License Agreement.
CLA signed

@markshannon
Copy link
Member

markshannon commented Dec 20, 2022

@penguin-wwy

Sorry, this got overlooked when I was updating the stats for LOAD_ATTR and CALL.

We still don't specify whether the failure is for a builtin function or method descriptor, so this PR would still be a valuable addition, if you are willing to update it.

@penguin-wwy
Copy link
Contributor Author

penguin-wwy commented Dec 21, 2022

I updated the failure for the method descriptor.

@@ -1251,10 +1251,10 @@ static int
function_kind(PyCodeObject *code) {
int flags = code->co_flags;
if ((flags & (CO_VARKEYWORDS | CO_VARARGS)) || code->co_kwonlyargcount) {
return SPEC_FAIL_CALL_COMPLEX_PARAMETERS;
return SPEC_FAIL_CODE_COMPLEX_PARAMETERS;
Copy link
Contributor Author

@penguin-wwy penguin-wwy Dec 22, 2022

Choose a reason for hiding this comment

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

function_kind used in subscrattr and call specialize, move the failure to the common category.

Copy link
Member

@markshannon markshannon left a comment

Thanks for doing this. There a couple more values that need changing.

Python/specialize.c Show resolved Hide resolved
Python/specialize.c Show resolved Hide resolved
@bedevere-bot
Copy link

bedevere-bot commented Dec 22, 2022

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@penguin-wwy
Copy link
Contributor Author

penguin-wwy commented Dec 22, 2022

I have made the requested changes; please review again.

@bedevere-bot
Copy link

bedevere-bot commented Dec 22, 2022

Thanks for making the requested changes!

@markshannon: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from markshannon Dec 22, 2022
Copy link
Member

@markshannon markshannon left a comment

That's great. Thanks.

@markshannon markshannon merged commit a021612 into python:main Dec 22, 2022
14 checks passed
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.

None yet

4 participants