feat(atoms): supports static atoms #6812
Conversation
|
|
||
| impl PartialEq for Atom { | ||
| fn eq(&self, other: &Self) -> bool { | ||
| // Fast path |
There was a problem hiding this comment.
@kdy1 After supporting Atom::static, I'm not sure the fast path will work as before. To keep the logic same as before, we need to call match twice. I'm going to remove the code, in case I miss something, what are your opinions?
There was a problem hiding this comment.
Also, should Atom promise assert!(Atom::Arc("foo") == Atom::Static("foo"))?
There was a problem hiding this comment.
I think it's fine, but assert!(Atom::Arc("foo") == Atom::Static("foo")) should work
| } | ||
|
|
||
| // fn _assert_size() { | ||
| // let _static_assert_size_eq = std::mem::transmute::<Atom, usize>; |
There was a problem hiding this comment.
@kdy1 The transmute seems not to work, after making Atom(ThinArc<HeaderWithLength<()>, u8>) to Atom(Inner).
Is this expected?
There was a problem hiding this comment.
Yes, but it will degrade performance greatly.
I didn't implement static support because it's hard
There was a problem hiding this comment.
I get the gist. We need something like ThinStaticStr. It's indeed hard after looking into thin_tr. I will try it, but it will take a while.
If anyone wants to try this, feel free to do it.
2 participants
Description:
This PR supports allowing
Atomto store&'static strin it.BREAKING CHANGE:
Related issue (if exists):
#4946