{"action":"closed","issue":{"url":"https://api.github.com/repos/servo/string-cache/issues/195","repository_url":"https://api.github.com/repos/servo/string-cache","labels_url":"https://api.github.com/repos/servo/string-cache/issues/195/labels{/name}","comments_url":"https://api.github.com/repos/servo/string-cache/issues/195/comments","events_url":"https://api.github.com/repos/servo/string-cache/issues/195/events","html_url":"https://github.com/servo/string-cache/issues/195","id":267498345,"number":195,"title":"Add a copiable atom type","user":{"login":"moulins","id":10963458,"avatar_url":"https://avatars1.githubusercontent.com/u/10963458?v=4","gravatar_id":"","url":"https://api.github.com/users/moulins","html_url":"https://github.com/moulins","followers_url":"https://api.github.com/users/moulins/followers","following_url":"https://api.github.com/users/moulins/following{/other_user}","gists_url":"https://api.github.com/users/moulins/gists{/gist_id}","starred_url":"https://api.github.com/users/moulins/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/moulins/subscriptions","organizations_url":"https://api.github.com/users/moulins/orgs","repos_url":"https://api.github.com/users/moulins/repos","events_url":"https://api.github.com/users/moulins/events{/privacy}","received_events_url":"https://api.github.com/users/moulins/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":4,"created_at":"2017-10-22T21:00:38Z","updated_at":"2017-10-23T11:08:31Z","closed_at":"2017-10-23T11:08:31Z","author_association":"NONE","body":"The `Atom` type is not `Copy` (from what I understand of the source code, because of the need to drop unused atoms), which make it not as usable as it could be.\r\n\r\nIn a lot of projects (like on-off CLI utilities, etc), we don't really care about dropping unused atoms, so we don't actually need the `Drop` implementation and `Atom` could in principle implement `Copy`.\r\n\r\nCurrently, I use my own type, which is a very hacky implementation of this idea:\r\n```rust\r\n//RawAtom is the type generated by string_cache\r\nuse interning_raw::RawAtom as RawAtom;\r\nuse std::ops::Deref;\r\nuse std;\r\n\r\n//A struct without the PhantomData field, so that we can implement Copy\r\n#[derive(Copy, Clone)]\r\npub struct Atom {\r\n\tpub unsafe_data: u64\r\n}\r\n\r\nimpl Deref for Atom {\r\n\ttype Target = str;\r\n\r\n\tfn deref(&self) -> &Self::Target {\r\n\t\tassert_eq!(std::mem::size_of::<Atom>(), std::mem::size_of::<RawAtom>());\r\n\t\t//Horribly unsafe, but it *should* work,\r\n\t\t//as the second field of RawAtom is zero-sized\r\n\t\t//and shouldn't affect the in-memory representation\r\n\t\tunsafe {\r\n\t\t\tstd::mem::transmute::<&Atom, &RawAtom>(self)\r\n\t\t}.deref()\r\n\t}\r\n}\r\n\r\nimpl<'a> From<&'a str> for Atom {\r\n\tfn from(val: &str) -> Atom {\r\n\t\tlet raw: RawAtom = val.into();\r\n\t\tlet atom = Atom { unsafe_data: raw.unsafe_data };\r\n\r\n\t\t//We leak the RawAtom, so that its (potential) storage isn't deallocated\r\n\t\tstd::mem::forget(raw);\r\n\t\tatom\r\n\t}\r\n}\r\n\r\n//Implementations of Eq, Hash, etc which mirror those in string_cache::atom\r\n```\r\nObviously, it's not ideal:\r\n- it's an awful hack, depending on implementation details;\r\n- I can't use the `atom!` macro, and if I define my own, I can't use it in pattern position:\r\n```rust\r\nmacro_rules! my_atom {\r\n\t($atom:expr) => {\r\n\t\t//This is not a pattern!\r\n\t\tatom!($atom).into::<$crate::interning::Atom>()\r\n\t}\r\n}\r\n```\r\nSo, here's my idea:\r\n- add a `CopiableAtom` type, with only the `unsafe_data` field, implementing `Copy`;\r\n- when constructing a dynamic `CopiableAtom`, leak the underlying data to ensure it will never be dropped;\r\n- add a knob to the `AtomType` builder to choose whether to create an `Atom` or a `CopiableAtom`; for example:\r\n```rust\r\nstring_cache_codegen::AtomType::new_copiable(\"foo::FooAtom\", \"foo_atom!\")...\r\n//or\r\nstring_cache_codegen::AtomType::new(\"foo::FooAtom\", \"foo_atom!\").copiable()...\r\n```\r\n"}}
{"actor":{"display_login":"SimonSapin"}}