Skip to content

Add sq_slots & mp_slots to instance (issue #1197) #1294

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.

Sign up for GitHub

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
Merged
merged 1 commit into from
Jul 18, 2016
Merged

Add sq_slots & mp_slots to instance (issue #1197) #1294

merged 1 commit into from
Jul 18, 2016

Conversation

sizeoftank
Copy link
Contributor

implement tp_as_sequence & tp_as_mapping for instance_cls

undingen
undingen reviewed Jul 14, 2016
View reviewed changes
src/runtime/classobj.cpp Outdated
AUTO_DECREF(getitem_func);
return runtimeCall(getitem_func, ArgPassSpec(1), key, NULL, NULL, NULL, NULL);
try {
return runtimeCall(getitem_func, ArgPassSpec(1), key, NULL, NULL, NULL, NULL);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can replace the whole try catch block with

return runtimeCallInternal<S, NOT_REWRITABLE>(getitem_func, NULL, ArgPassSpec(1), key, NULL, NULL, NULL, NULL);

@undingen
Copy link
Contributor

thanks for the patch! 👍
I added a few comments, I'm a little bit worried about the additional code duplication this introduces but it does not make things much worse than the old class support already is :-)
I guess we really just have to replace the whole implementation some point in the future with cpythons but for now this is a improvement.

@sizeoftank
Copy link
Contributor Author

sizeoftank commented Jul 14, 2016 •
edited
Loading

thanks for your response! 👍
oh, these issues aroused by my careless, I make a change to fix them. :-)
I also think we should refactor our codes to make it better compatibility with cpython, but I'm not sure I can complete these works correctly now.

undingen
undingen reviewed Jul 14, 2016
View reviewed changes
src/runtime/classobj.cpp
static BoxedString* delslice_str, *setslice_str, *delitem_str, *setitem_str;
Box* func = NULL, * res = NULL;
Box* slice = NULL;
Box* begin = boxInt(i);
Copy link
Contributor

Choose a reason for hiding this comment

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

begin and end will get leaked at the return -1 statments.
better use AUTO_DECREF(begin);

@sizeoftank sizeoftank force-pushed the pr_instance_sqslots_mpslots_issue1197 branch from b1f5623 to 0c37fa0 Compare July 18, 2016 13:16
@sizeoftank
Copy link
Contributor Author

sizeoftank commented Jul 18, 2016 •
edited
Loading

I stash these into one
Are there any other need to do？

@undingen undingen merged commit 6a33d15 into pyston:master Jul 18, 2016
@undingen
Copy link
Contributor

LGTM, thanks for the patch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sizeoftank @undingen