From 63829a2a9f8cec9690da59541d68073a97994f35 Mon Sep 17 00:00:00 2001 From: "Sergey \"Shnatsel\" Davidoff" Date: Sat, 2 Nov 2019 18:33:17 +0100 Subject: [PATCH 1/4] Drop unnecessary `unsafe` --- src/macros.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index 5662d289..19c7dcfa 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -470,19 +470,16 @@ macro_rules! crate_authors { impl Deref for CargoAuthors { type Target = str; - #[allow(unsafe_code)] fn deref(&self) -> &'static str { static ONCE: Once = Once::new(); static mut VALUE: *const String = 0 as *const String; - unsafe { - ONCE.call_once(|| { - let s = env!("CARGO_PKG_AUTHORS").replace(':', $sep); - VALUE = Box::into_raw(Box::new(s)); - }); + ONCE.call_once(|| { + let s = env!("CARGO_PKG_AUTHORS").replace(':', $sep); + VALUE = Box::leak(s); + }); - &(*VALUE)[..] - } + &(*VALUE)[..] } } From 0085bb7ee2f4ab7aac4797c2552adb1e004ced08 Mon Sep 17 00:00:00 2001 From: "Sergey \"Shnatsel\" Davidoff" Date: Sat, 2 Nov 2019 19:48:16 +0100 Subject: [PATCH 2/4] Make code actually work --- src/macros.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index 19c7dcfa..95233475 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -459,7 +459,7 @@ macro_rules! crate_version { macro_rules! crate_authors { ($sep:expr) => {{ use std::ops::Deref; - use std::sync::Once; + use std::boxed::Box; #[allow(missing_copy_implementations)] #[allow(dead_code)] @@ -471,15 +471,9 @@ macro_rules! crate_authors { type Target = str; fn deref(&self) -> &'static str { - static ONCE: Once = Once::new(); - static mut VALUE: *const String = 0 as *const String; - - ONCE.call_once(|| { - let s = env!("CARGO_PKG_AUTHORS").replace(':', $sep); - VALUE = Box::leak(s); - }); - - &(*VALUE)[..] + let s: Box = Box::new(env!("CARGO_PKG_AUTHORS").replace(':', $sep)); + let s2 = Box::leak(s); + &*s2 // weird but compiler-suggested way to turn a String into &str } } From 925956d594cac2cfcfc801179d537b1ad9ac99ca Mon Sep 17 00:00:00 2001 From: "Sergey \"Shnatsel\" Davidoff" Date: Sat, 2 Nov 2019 20:18:57 +0100 Subject: [PATCH 3/4] Do not leak a new string on every invocation of crate_authors! --- src/macros.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index 95233475..d2446729 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -460,10 +460,12 @@ macro_rules! crate_authors { ($sep:expr) => {{ use std::ops::Deref; use std::boxed::Box; + use std::cell::Cell; #[allow(missing_copy_implementations)] #[allow(dead_code)] struct CargoAuthors { + authors: Cell>, __private_field: (), }; @@ -471,13 +473,22 @@ macro_rules! crate_authors { type Target = str; fn deref(&self) -> &'static str { - let s: Box = Box::new(env!("CARGO_PKG_AUTHORS").replace(':', $sep)); - let s2 = Box::leak(s); - &*s2 // weird but compiler-suggested way to turn a String into &str + let authors = self.authors.take(); + if authors.is_some() { + let unwrapped_authors = authors.unwrap(); + self.authors.replace(Some(unwrapped_authors)); + unwrapped_authors + } else { + let s: Box = Box::new(env!("CARGO_PKG_AUTHORS").replace(':', $sep)); + let static_string = Box::leak(s); + self.authors.replace(Some(&*static_string)); + &*static_string // weird but compiler-suggested way to turn a String into &str + } } } &*CargoAuthors { + authors: std::cell::Cell::new(Option::None), __private_field: (), } }}; From 0a010a4371c8765996efd943b5240daab6fbf9c6 Mon Sep 17 00:00:00 2001 From: "Sergey \"Shnatsel\" Davidoff" Date: Sat, 2 Nov 2019 21:03:35 +0100 Subject: [PATCH 4/4] Add comment on performance --- src/macros.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/macros.rs b/src/macros.rs index d2446729..03e518b0 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -479,6 +479,9 @@ macro_rules! crate_authors { self.authors.replace(Some(unwrapped_authors)); unwrapped_authors } else { + // This caches the result for subsequent invocations of the same instance of the macro + // to avoid performing one memory allocation per call. + // If performance ever becomes a problem for this code, it should be moved to build.rs let s: Box = Box::new(env!("CARGO_PKG_AUTHORS").replace(':', $sep)); let static_string = Box::leak(s); self.authors.replace(Some(&*static_string));