Allow any mapping for locals.#3314
Conversation
DimitrisJim
force-pushed
the
mapping_locals
branch
2 times, most recently
from
6361512 to
067bb73
Compare
| 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.
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.
Oh, not that simple due to sequence protocol.
There was a problem hiding this comment.
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.
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.
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.
DimitrisJim
force-pushed
the
mapping_locals
branch
from
067bb73 to
895b52e
Compare
DimitrisJim
force-pushed
the
mapping_locals
branch
from
895b52e to
94b9163
Compare
2 participants
No description provided.