-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Allow any mapping for locals. #3314
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
Conversation
6361512 to
067bb73
Compare
vm/src/frame.rs
Outdated
| bytecode::Instruction::DeleteLocal(idx) => { | ||
| let name = &self.code.names[*idx as usize]; | ||
| match self.locals.del_item(name.clone(), vm) { | ||
| match self.locals.clone().into_object().del_item(name.clone(), vm) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is calling del_item from ItemProtocol. The ideal way to do this would be impl ItemProtocol for PyBuffer. Then self.locals.del_item will work.
Actually, most of the code from ItemProtocol for PyObjectRef looks PyMapping's.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, not that simple due to sequence protocol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, when #3316 lands, get/set/del item should be amended to try and and also use the sequence protocol, as CPython does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but I didn't use the check here to try the dict method first (which is what is going to get used most of the times), I'll add that now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After reviewing ItemProtocol, I now feel like it is almost useless. ItemProcotol for PyDict includes the dict-first branch. Because it is there, we cannot reuse the code here.
067bb73 to
895b52e
Compare
895b52e to
94b9163
Compare
No description provided.