ZeroTrie: change map_store to pub convert_store #4022
Conversation
There was a problem hiding this comment.
For context: #2722 (comment)
Doesn't NewStore::from(trie.take_store()).into() work without naming the type?
experimental/zerotrie/src/serde.rs
Outdated
| _ => return Err(D::Error::custom("invalid ZeroTrie tag")), | ||
| }; | ||
| let zerotrie = match *tag { | ||
| tags::SIMPLE_ASCII => ZeroTrieSimpleAscii::from_store(trie_bytes) |
There was a problem hiding this comment.
Does this need to use cast_store? We have a store, so why not convert and then create the trie, instead of create and then convert.
An extra type inference is needed when calling |
Are you happy with the method name? It's not really a "cast" in the sense that it runs actual code, not just a type system inference (static cast) or type id check (dynamic cast). This is a converting cast invoking the From impl. Bikeshed:
Maybe convert_store is the best. It's not an entirely new term to ICU4X but it's fairly specific to the circumstance when used. |
|
a From impl is actual code. I don't mind as long as it's well documented |
|
cargo fmt. :( autosubmit = on |
Toward #2909
I think @robertbastian did not like the
map_storefunction, but I needed it again, and this time outside the crate. I made a new function namedcast_storethat requires aFromimpl and made it public. Happy to choose some other name.Although this logic is possible to write outside this crate, I prefer to keep it in this crate. Here is how one could write this logic without this function:
Note that the trie type needs to be named. With this PR, you can simply do: