Allow any mapping for locals. #3314
Conversation
6361512 to
067bb73
Compare
vm/src/frame.rs
Outdated
| @@ -559,7 +578,7 @@ impl ExecutingFrame<'_> { | |||
| } | |||
| 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.
067bb73 to
895b52e
Compare
895b52e to
94b9163
Compare
No description provided.