Implement dictionary indexing by trait. #1187
Conversation
|
|
||
| /// Implement trait for the str type, so that we can use strings | ||
| /// to index dictionaries. | ||
| impl DictKey for String { |
There was a problem hiding this comment.
| impl DictKey for String { | |
| impl DictKey for str { |
There was a problem hiding this comment.
I tried this, without success. I get this compilation error:
--> vm/src/dictdatatype.rs:363:24
|
363 | let val = dict.get(&vm, "x").unwrap().unwrap();
| ^^^ doesn't have a size known at compile-time
There was a problem hiding this comment.
Oh, I think it's because it's trying to make a double-fat pointer, with both a vtable and a slice length, and it doesn't like that.
vm/src/dictdatatype.rs
Outdated
| fn do_eq(&self, vm: &VirtualMachine, other_key: &PyObjectRef) -> PyResult<bool> { | ||
| // Fall back to PyString implementation. | ||
| let s = vm.new_str(self.to_string()); | ||
| s.do_eq(vm, other_key) |
There was a problem hiding this comment.
I feel like we could have a more efficient eq implementation here than reallocating the string.
There was a problem hiding this comment.
Absolutely, this involves checking the specific python type to be of PyStringRef, and then doing a string compare. I suspect this happens only in a couple of cases, after the first hash compare is a success. I will implement this though, since I think it is a good idea.
windelbouwman
force-pushed
the
dict-keying
branch
from
f2a70bb to
42dca44
Compare
This add support for indexing the dictionary by string. So instead of creating a PyStringRef, we now can use rust's String type to index a dict.