From 6a6de046eb99026c1812d7d92cdb40ca962891f2 Mon Sep 17 00:00:00 2001 From: Troy Benson Date: Sat, 8 Jul 2023 17:46:09 +0000 Subject: [PATCH] fix: unsoundness with refs --- common/src/config.rs | 6 +- config/config/example/derive.rs | 22 +- config/config/src/key.rs | 160 +++++------- config/config/src/lib.rs | 6 +- config/config/src/primitives.rs | 401 +++++++++++-------------------- config/config/src/sources/cli.rs | 10 +- config/config/src/sources/env.rs | 4 +- config/config/src/tests.rs | 16 +- config/config_derive/src/lib.rs | 7 +- 9 files changed, 242 insertions(+), 390 deletions(-) diff --git a/common/src/config.rs b/common/src/config.rs index de768967..07b0a988 100644 --- a/common/src/config.rs +++ b/common/src/config.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use anyhow::Result; use crate::logging; @@ -30,8 +32,8 @@ pub struct LoggingConfig { } impl ::config::Config for logging::Mode { - fn graph() -> &'static ::config::KeyGraph { - &::config::KeyGraph::String + fn graph() -> Arc<::config::KeyGraph> { + Arc::new(::config::KeyGraph::String) } } diff --git a/config/config/example/derive.rs b/config/config/example/derive.rs index 66bceda2..50aa6b26 100644 --- a/config/config/example/derive.rs +++ b/config/config/example/derive.rs @@ -21,26 +21,12 @@ struct LoggingConfig { manual2: Manual, } -#[derive(Default, Debug, PartialEq, serde::Deserialize)] +#[derive(config::Config, Default, Debug, PartialEq, serde::Deserialize)] struct Manual { + #[config(cli(skip), env(skip))] cycle: Box>, } -impl config::Config for Manual { - fn graph() -> &'static config::KeyGraph { - if let Some(tree) = config::KeyGraph::get::() { - return tree; - } - - let mut keys = std::collections::BTreeMap::new(); - - keys.insert("level".to_string(), config::Key::new(String::graph())); - keys.insert("json".to_string(), config::Key::new(bool::graph())); - - config::KeyGraph::store::(config::KeyGraph::Struct(keys)) - } -} - impl Default for LoggingConfig { fn default() -> Self { Self { @@ -60,9 +46,7 @@ fn main() { } fn parse() -> Result { - let graph = std::thread::spawn(AppConfig::graph).join().unwrap(); - - println!("{:#?}", graph); + dbg!(AppConfig::graph()); let mut builder = ConfigBuilder::new(); builder.add_source(sources::CliSource::new()?); diff --git a/config/config/src/key.rs b/config/config/src/key.rs index 9c3d3e4b..cc238f7e 100644 --- a/config/config/src/key.rs +++ b/config/config/src/key.rs @@ -1,8 +1,7 @@ use std::{ - collections::{btree_map::Entry, BTreeMap}, + collections::{BTreeMap, HashMap}, fmt::Display, - ptr::NonNull, - sync::Mutex, + sync::{Mutex, Arc, Weak}, any::TypeId, }; #[derive(Debug, Default, Clone, PartialEq, Eq, Hash)] @@ -95,85 +94,50 @@ pub enum KeyType { Struct(BTreeMap), Map(Box, Box), Seq(Box), - CyclicReference, + CyclicReference(&'static str), } -static MEMO_GRAPHS: Mutex> = - Mutex::new(BTreeMap::new()); - -struct KeyGraphRefHelper { - ptr: NonNull, - ref_key: NonNull, - is_building: bool, +thread_local! { + static MEMO_GRAPHS: Mutex, bool)>> = Mutex::new(HashMap::new()); } -// Safety: The pointer is never null and is static. -// The pointer is also never dropped. And the pointer does not contain -// any data that can be dataraced as it is locked behind a mutex. -unsafe impl Send for KeyGraphRefHelper {} - -impl Drop for KeyGraphRefHelper { - fn drop(&mut self) { - // Safety: We are the only ones with access to the pointer. - // We are dropping the pointer and therefore we can drop the value. - // In practice this should never actually be called because the `MEMO_GRAPHS` is a static - // and therefore will never be dropped. - unsafe { - let _ = Box::from_raw(self.ptr.as_ptr()); - let _ = Box::from_raw(self.ref_key.as_ptr()); - } - } +pub struct KeyGraphBuilder { + graph: Arc, + alive: bool, + building: bool, + phantom: std::marker::PhantomData, } -impl KeyGraphRefHelper { - fn new() -> Self { - // We create a pointer to some dummy value. - let ptr = NonNull::new(Box::leak(Box::new(KeyGraph::Unit))).unwrap(); - - // We then create a pointer to a ref to the dummy value. - // This is because the reference key is a cyclic reference. - // And we if we are calling the get function from the same path that is building the - // tree then we need to return the ref key. Otherwise we would run into an infinite - // loop and stack overflow. - // Safety: We created a non null pointer above and its static. - let ref_key = - NonNull::new(Box::leak(Box::new(KeyGraph::Ref(unsafe { ptr.as_ref() })))).unwrap(); - - Self { - ref_key, - is_building: true, - ptr, +impl KeyGraphBuilder { + pub fn get(&self) -> Option> { + if self.building { + Some(Arc::new(KeyGraph::Ref(self.graph.clone(), std::any::type_name::()))) + } else if self.alive { + Some(self.graph.clone()) + } else { + None } } - fn get(&mut self) -> &'static KeyGraph { - if self.is_building { - // The reason we return the ref key here is because if the get function is being - // called by the same path that is building the tree then we need to return the - // ref key because this is a cyclic reference. - // Safety: The value is non-null and is static. We also have a mutable ref to `KeyGraphRefHelper` - unsafe { self.ref_key.as_ref() } - } else { - // If we are not building then we can return the actual pointer, ie: memoization. - // Safety: The value is non-null and is static. We also have a mutable ref to `KeyGraphRefHelper` - unsafe { self.ptr.as_ref() } + pub fn build(self, graph: KeyGraph) -> Arc { + if let Some(arc) = self.get() { + return arc; } - } - fn set(&mut self, mut tree: KeyGraph) { - // Safety: We have a mutable ref to `KeyGraphRefHelper` and therefore we are the only - // ones with access to the pointer. - // We swap the old value of the pointer with the new value. We can then drop the old - // value of the pointer. (which will be in `tree`) - // It should be impossible to call this function twice on the same type. - // However even if it is called twice it is still safe because we are swapping the - // pointer and therefore the old value will be dropped and no memory leaks will occur. unsafe { - std::mem::swap(self.ptr.as_mut(), &mut tree); + let graph_ptr = self.graph.as_ref() as *const KeyGraph as *mut KeyGraph; + let _ = std::mem::replace(&mut *graph_ptr, graph); } - // Since we are no longer building and are built we stop returning the ref key. - self.is_building = false; + MEMO_GRAPHS.with(|mg| { + let mut mg = mg.lock().unwrap(); + let ty = TypeId::of::(); + if let Some((_, building)) = mg.get_mut(&ty) { + *building = false; + } + }); + + self.graph } } @@ -193,11 +157,11 @@ pub enum KeyGraph { Bool, Unit, Char, - Option(&'static KeyGraph), + Option(Arc), Struct(BTreeMap), - Map(&'static KeyGraph, &'static KeyGraph), - Seq(&'static KeyGraph), - Ref(&'static KeyGraph), + Map(Arc, Arc), + Seq(Arc), + Ref(Arc, &'static str), } impl std::fmt::Debug for KeyGraph { @@ -221,7 +185,7 @@ impl std::fmt::Debug for KeyGraph { Self::Struct(map) => write!(f, "Struct({:?})", map), Self::Map(key, value) => write!(f, "Map({:?}, {:?})", key, value), Self::Seq(key) => write!(f, "Seq({:?})", key), - Self::Ref(_) => write!(f, "Ref(...)"), + Self::Ref(_, ty) => write!(f, "Ref(&{})", ty), } } } @@ -251,30 +215,38 @@ impl KeyGraph { KeyType::Map(Box::new(key.key_type()), Box::new(value.key_type())) } Self::Seq(key) => KeyType::Seq(Box::new(key.key_type())), - Self::Ref(_) => KeyType::CyclicReference, + Self::Ref(_, ty) => KeyType::CyclicReference(ty), } } - pub fn get() -> Option<&'static Self> { - let mut memo_graphs = MEMO_GRAPHS.lock().expect("Failed to lock memo tree"); - let id = std::any::TypeId::of::(); - - if let Entry::Vacant(e) = memo_graphs.entry(id) { - e.insert(KeyGraphRefHelper::new()); - None - } else { - memo_graphs.get_mut(&id).map(|h| h.get()) - } - } + pub fn builder() -> KeyGraphBuilder { + MEMO_GRAPHS.with(|mg| { + let mut mg = mg.lock().unwrap(); + + let ty = TypeId::of::(); + if let Some((graph, building)) = mg.get(&ty) { + if let Some(graph) = graph.upgrade() { + return KeyGraphBuilder { + graph, + alive: true, + building: *building, + phantom: std::marker::PhantomData, + }; + } + } - pub fn store(tree: Self) -> &'static Self { - let mut memo_graphs = MEMO_GRAPHS.lock().expect("Failed to lock memo tree"); + // Dummy value does not matter + let graph = Arc::new(KeyGraph::Unit); - let id = std::any::TypeId::of::(); - let helper = memo_graphs.get_mut(&id).unwrap(); + mg.insert(ty, (Arc::downgrade(&graph), true)); - helper.set(tree); - helper.get() + KeyGraphBuilder { + graph, + alive: false, + building: false, + phantom: std::marker::PhantomData, + } + }) } } @@ -283,14 +255,14 @@ impl KeyGraph { /// - type #[derive(Clone, Debug)] pub struct Key { - tree: &'static KeyGraph, + tree: Arc, skip_cli: bool, skip_env: bool, comment: Option<&'static str>, } impl Key { - pub fn new(tree: &'static KeyGraph) -> Self { + pub fn new(tree: Arc) -> Self { Self { tree, skip_cli: false, @@ -327,7 +299,7 @@ impl Key { } pub fn tree(&self) -> &KeyGraph { - self.tree + &self.tree } pub fn key_type(&self) -> KeyType { diff --git a/config/config/src/lib.rs b/config/config/src/lib.rs index 141ac628..4048dd26 100644 --- a/config/config/src/lib.rs +++ b/config/config/src/lib.rs @@ -187,10 +187,10 @@ pub trait Config: serde::de::DeserializeOwned + 'static { const VERSION: Option<&'static str> = None; const AUTHOR: Option<&'static str> = None; - fn graph() -> &'static KeyGraph; + fn graph() -> Arc; fn validate(value: Value) -> Result { - validate_from_graph(Self::graph(), value) + validate_from_graph(&Self::graph(), value) } } @@ -290,7 +290,7 @@ pub fn validate_from_graph(tree: &KeyGraph, value: Value) -> Result { ) } } - KeyGraph::Ref(tree) => validate_from_graph(tree, value), + KeyGraph::Ref(tree, _) => validate_from_graph(tree, value), } } diff --git a/config/config/src/primitives.rs b/config/config/src/primitives.rs index d9238af4..6995c367 100644 --- a/config/config/src/primitives.rs +++ b/config/config/src/primitives.rs @@ -14,8 +14,8 @@ use crate::{Config, ConfigErrorEnum, KeyGraph, Result, Value}; macro_rules! impl_config_primitive { ($ty:ty, $kt:expr, |$value:ident| $valid:expr) => { impl Config for $ty { - fn graph() -> &'static KeyGraph { - &$kt + fn graph() -> Arc { + Arc::new($kt) } fn validate($value: Value) -> Result { @@ -208,11 +208,11 @@ impl_config_primitive!(String, KeyGraph::String, |value| { impl_config_primitive!((), KeyGraph::Unit, |_value| Ok(Value::Unit)); impl Config for isize { - fn graph() -> &'static KeyGraph { + fn graph() -> Arc { #[cfg(target_pointer_width = "32")] - return &KeyGraph::I32; + return Arc::new(KeyGraph::I32); #[cfg(target_pointer_width = "64")] - return &KeyGraph::I64; + return Arc::new(KeyGraph::I64); } fn validate(value: Value) -> Result { @@ -224,11 +224,11 @@ impl Config for isize { } impl Config for usize { - fn graph() -> &'static KeyGraph { + fn graph() -> Arc { #[cfg(target_pointer_width = "32")] - return &KeyGraph::U32; + return Arc::new(KeyGraph::U32); #[cfg(target_pointer_width = "64")] - return &KeyGraph::U64; + return Arc::new(KeyGraph::U64); } fn validate(value: Value) -> Result { @@ -242,12 +242,13 @@ impl Config for usize { // Special Type impl Config for Option { - fn graph() -> &'static KeyGraph { - if let Some(graph) = KeyGraph::get::() { + fn graph() -> Arc { + let builder = KeyGraph::builder::(); + if let Some(graph) = builder.get() { return graph; } - KeyGraph::store::(KeyGraph::Option(C::graph())) + builder.build(KeyGraph::Option(C::graph())) } fn validate(value: Value) -> Result { @@ -262,8 +263,8 @@ impl Config for Option { } impl Config for Box { - fn graph() -> &'static KeyGraph { - &KeyGraph::String + fn graph() -> Arc { + Arc::new(KeyGraph::String) } fn validate(value: Value) -> Result { @@ -274,7 +275,7 @@ impl Config for Box { // Transparent Types impl Config for Box { - fn graph() -> &'static KeyGraph { + fn graph() -> Arc { C::graph() } @@ -284,7 +285,7 @@ impl Config for Box { } impl Config for Cell { - fn graph() -> &'static KeyGraph { + fn graph() -> Arc { C::graph() } @@ -294,7 +295,7 @@ impl Config for Cell { } impl Config for RefCell { - fn graph() -> &'static KeyGraph { + fn graph() -> Arc { C::graph() } @@ -304,7 +305,7 @@ impl Config for RefCell { } impl Config for Mutex { - fn graph() -> &'static KeyGraph { + fn graph() -> Arc { C::graph() } @@ -314,7 +315,7 @@ impl Config for Mutex { } impl Config for Rc { - fn graph() -> &'static KeyGraph { + fn graph() -> Arc { C::graph() } @@ -324,7 +325,7 @@ impl Config for Rc { } impl Config for Arc { - fn graph() -> &'static KeyGraph { + fn graph() -> Arc { C::graph() } @@ -334,7 +335,7 @@ impl Config for Arc { } impl Config for RwLock { - fn graph() -> &'static KeyGraph { + fn graph() -> Arc { C::graph() } @@ -344,8 +345,8 @@ impl Config for RwLock { } impl Config for PhantomData { - fn graph() -> &'static KeyGraph { - &KeyGraph::Unit + fn graph() -> Arc { + Arc::new(KeyGraph::Unit) } fn validate(value: Value) -> Result { @@ -355,63 +356,58 @@ impl Config for PhantomData { // Sequence Types -impl Config for BTreeSet { - fn graph() -> &'static KeyGraph { - if let Some(graph) = KeyGraph::get::() { - return graph; - } +pub struct Seq(PhantomData); - KeyGraph::store::(KeyGraph::Seq(C::graph())) +fn seq_graph() -> Arc { + let builder = KeyGraph::builder::>(); + if let Some(graph) = builder.get() { + return graph; } - fn validate(value: Value) -> Result { - match value { - Value::Seq(seq) => { - let mut result = BTreeSet::new(); - for value in seq { - let value = C::validate(value)?; - result.insert(value); - } + builder.build(KeyGraph::Seq(C::graph())) +} - Ok(Value::Seq(result.into_iter().collect())) +fn seq_validate(value: Value) -> Result { + match value { + Value::Seq(seq) => { + let mut result = BTreeSet::new(); + for value in seq { + let value = C::validate(value)?; + result.insert(value); } - _ => Err(ConfigErrorEnum::ValidationError(format!( - "{:?} is not convertable into BTreeSet<{}>", - value, - stringify!(C) - )) - .into()), + + Ok(Value::Seq(result.into_iter().collect())) } + _ => Err(ConfigErrorEnum::ValidationError(format!( + "{:?} is not convertable into {}", + value, + std::any::type_name::() + )) + .into()), } } -impl Config for BinaryHeap { - fn graph() -> &'static KeyGraph { - if let Some(graph) = KeyGraph::get::() { - return graph; - } - - KeyGraph::store::(KeyGraph::Seq(C::graph())) +impl Config for BTreeSet { + #[inline(always)] + fn graph() -> Arc { + seq_graph::() } + #[inline(always)] fn validate(value: Value) -> Result { - match value { - Value::Seq(seq) => { - let mut result = BinaryHeap::new(); - for value in seq { - let value = C::validate(value)?; - result.push(value); - } + seq_validate::(value) + } +} - Ok(Value::Seq(result.into_iter().collect())) - } - _ => Err(ConfigErrorEnum::ValidationError(format!( - "{:?} is not convertable into BinaryHeap<{}>", - value, - stringify!(C) - )) - .into()), - } +impl Config for BinaryHeap { + #[inline(always)] + fn graph() -> Arc { + seq_graph::() + } + + #[inline(always)] + fn validate(value: Value) -> Result { + seq_validate::(value) } } @@ -420,186 +416,104 @@ impl< S: std::hash::BuildHasher + std::default::Default + 'static, > Config for HashSet { - fn graph() -> &'static KeyGraph { - if let Some(graph) = KeyGraph::get::() { - return graph; - } - - KeyGraph::store::(KeyGraph::Seq(C::graph())) + #[inline(always)] + fn graph() -> Arc { + seq_graph::() } fn validate(value: Value) -> Result { - match value { - Value::Seq(seq) => { - let mut result = HashSet::new(); - for value in seq { - let value = C::validate(value)?; - result.insert(value); - } - - Ok(Value::Seq(result.into_iter().collect())) - } - _ => Err(ConfigErrorEnum::ValidationError(format!( - "{:?} is not convertable into HashSet<{}>", - value, - stringify!(C) - )) - .into()), - } + seq_validate::(value) } } impl Config for LinkedList { - fn graph() -> &'static KeyGraph { - if let Some(graph) = KeyGraph::get::() { - return graph; - } - - KeyGraph::store::(KeyGraph::Seq(C::graph())) + #[inline(always)] + fn graph() -> Arc { + seq_graph::() } fn validate(value: Value) -> Result { - match value { - Value::Seq(seq) => { - let mut result = LinkedList::new(); - for value in seq { - let value = C::validate(value)?; - result.push_back(value); - } - - Ok(Value::Seq(result.into_iter().collect())) - } - _ => Err(ConfigErrorEnum::ValidationError(format!( - "{:?} is not convertable into LinkedList<{}>", - value, - stringify!(C) - )) - .into()), - } + seq_validate::(value) } } impl Config for VecDeque { - fn graph() -> &'static KeyGraph { - if let Some(graph) = KeyGraph::get::() { - return graph; - } - - KeyGraph::store::(KeyGraph::Seq(C::graph())) + #[inline(always)] + fn graph() -> Arc { + seq_graph::() } fn validate(value: Value) -> Result { - match value { - Value::Seq(seq) => { - let mut result = Vec::new(); - for value in seq { - let value = C::validate(value)?; - result.push(value); - } - - Ok(Value::Seq(result)) - } - _ => Err(ConfigErrorEnum::ValidationError(format!( - "{:?} is not convertable into VecDeque<{}>", - value, - stringify!(C) - )) - .into()), - } + seq_validate::(value) } } impl Config for Vec { - fn graph() -> &'static KeyGraph { - if let Some(graph) = KeyGraph::get::() { - return graph; - } - - KeyGraph::store::(KeyGraph::Seq(C::graph())) + #[inline(always)] + fn graph() -> Arc { + seq_graph::() } fn validate(value: Value) -> Result { - match value { - Value::Seq(seq) => { - let mut result = Vec::new(); - for value in seq { - let value = C::validate(value)?; - result.push(value); - } - - Ok(Value::Seq(result)) - } - _ => Err(ConfigErrorEnum::ValidationError(format!( - "{:?} is not convertable into Vec<{}>", - value, - stringify!(C) - )) - .into()), - } + seq_validate::(value) } } impl Config for Box<[C]> { - fn graph() -> &'static KeyGraph { - if let Some(graph) = KeyGraph::get::() { - return graph; - } - - KeyGraph::store::(KeyGraph::Seq(C::graph())) + #[inline(always)] + fn graph() -> Arc { + seq_graph::() } + #[inline(always)] fn validate(value: Value) -> Result { - match value { - Value::Seq(seq) => { - let mut result = Vec::new(); - for value in seq { - let value = C::validate(value)?; - result.push(value); - } - - Ok(Value::Seq(result)) - } - _ => Err(ConfigErrorEnum::ValidationError(format!( - "{:?} is not convertable into [{}]", - value, - stringify!(C) - )) - .into()), - } + seq_validate::(value) } } // Map Types -impl Config for BTreeMap { - fn graph() -> &'static KeyGraph { - if let Some(graph) = KeyGraph::get::() { - return graph; - } +pub struct Map(PhantomData, PhantomData); - KeyGraph::store::(KeyGraph::Map(K::graph(), V::graph())) +fn map_graph() -> Arc { + let builder = KeyGraph::builder::>(); + if let Some(graph) = builder.get() { + return graph; } - fn validate(value: Value) -> Result { - match value { - Value::Map(map) => { - let mut result = BTreeMap::new(); - for (key, value) in map { - let key = K::validate(key)?; - let value = V::validate(value)?; - result.insert(key, value); - } + builder.build(KeyGraph::Map(K::graph(), V::graph())) +} - Ok(Value::Map(result)) +fn map_validate(value: Value) -> Result { + match value { + Value::Map(map) => { + let mut result = BTreeMap::new(); + for (key, value) in map { + let key = K::validate(key)?; + let value = V::validate(value)?; + result.insert(key, value); } - _ => Err(ConfigErrorEnum::ValidationError(format!( - "{:?} is not convertable into BTreeMap<{}, {}>", - value, - stringify!(K), - stringify!(V) - )) - .into()), + + Ok(Value::Map(result)) } + _ => Err(ConfigErrorEnum::ValidationError(format!( + "{:?} is not convertable into {}", + value, + std::any::type_name::() + )) + .into()), + } +} + +impl Config for BTreeMap { + #[inline(always)] + fn graph() -> Arc { + map_graph::() + } + + #[inline(always)] + fn validate(value: Value) -> Result { + map_validate::(value) } } @@ -609,42 +523,22 @@ impl< S: std::hash::BuildHasher + std::default::Default + 'static, > Config for HashMap { - fn graph() -> &'static KeyGraph { - if let Some(graph) = KeyGraph::get::() { - return graph; - } - - KeyGraph::store::(KeyGraph::Map(K::graph(), V::graph())) + #[inline(always)] + fn graph() -> Arc { + map_graph::() } + #[inline(always)] fn validate(value: Value) -> Result { - match value { - Value::Map(map) => { - let mut result = BTreeMap::new(); - for (key, value) in map { - let key = K::validate(key)?; - let value = V::validate(value)?; - result.insert(key, value); - } - - Ok(Value::Map(result)) - } - _ => Err(ConfigErrorEnum::ValidationError(format!( - "{:?} is not convertable into HashMap<{}, {}>", - value, - stringify!(K), - stringify!(V) - )) - .into()), - } + map_validate::(value) } } // Network Types impl Config for IpAddr { - fn graph() -> &'static KeyGraph { - &KeyGraph::String + fn graph() -> Arc { + Arc::new(KeyGraph::String) } fn validate(value: Value) -> Result { @@ -666,8 +560,8 @@ impl Config for IpAddr { } impl Config for Ipv4Addr { - fn graph() -> &'static KeyGraph { - &KeyGraph::String + fn graph() -> Arc { + Arc::new(KeyGraph::String) } fn validate(value: Value) -> Result { @@ -692,8 +586,8 @@ impl Config for Ipv4Addr { } impl Config for Ipv6Addr { - fn graph() -> &'static KeyGraph { - &KeyGraph::String + fn graph() -> Arc { + Arc::new(KeyGraph::String) } fn validate(value: Value) -> Result { @@ -718,8 +612,8 @@ impl Config for Ipv6Addr { } impl Config for SocketAddr { - fn graph() -> &'static KeyGraph { - &KeyGraph::String + fn graph() -> Arc { + Arc::new(KeyGraph::String) } fn validate(value: Value) -> Result { @@ -744,8 +638,8 @@ impl Config for SocketAddr { } impl Config for SocketAddrV4 { - fn graph() -> &'static KeyGraph { - &KeyGraph::String + fn graph() -> Arc { + Arc::new(KeyGraph::String) } fn validate(value: Value) -> Result { @@ -770,8 +664,8 @@ impl Config for SocketAddrV4 { } impl Config for SocketAddrV6 { - fn graph() -> &'static KeyGraph { - &KeyGraph::String + fn graph() -> Arc { + Arc::new(KeyGraph::String) } fn validate(value: Value) -> Result { @@ -798,8 +692,8 @@ impl Config for SocketAddrV6 { // Other Miscellaneous Types impl Config for PathBuf { - fn graph() -> &'static KeyGraph { - &KeyGraph::String + fn graph() -> Arc { + Arc::new(KeyGraph::String) } fn validate(value: Value) -> Result { @@ -808,8 +702,8 @@ impl Config for PathBuf { } impl Config for Duration { - fn graph() -> &'static KeyGraph { - &KeyGraph::String + fn graph() -> Arc { + Arc::new(KeyGraph::String) } fn validate(value: Value) -> Result { @@ -834,8 +728,8 @@ impl Config for Duration { } impl Config for SystemTime { - fn graph() -> &'static KeyGraph { - &KeyGraph::String + fn graph() -> Arc { + Arc::new(KeyGraph::String) } fn validate(value: Value) -> Result { @@ -864,14 +758,9 @@ impl Config for SystemTime { macro_rules! impl_slice { ($size:expr) => { impl Config for [C; $size] { - fn graph() -> &'static KeyGraph { - if let Some(graph) = KeyGraph::get::() { - return graph; - } - - KeyGraph::store::(KeyGraph::Seq( - C::graph(), - )) + #[inline(always)] + fn graph() -> Arc { + seq_graph::() } fn validate(value: Value) -> Result { @@ -897,7 +786,7 @@ macro_rules! impl_slice { _ => Err(ConfigErrorEnum::ValidationError(format!( "{:?} is not convertable into [{}; {}]", value, - stringify!(C), + std::any::type_name::(), $size )) .into()), diff --git a/config/config/src/sources/cli.rs b/config/config/src/sources/cli.rs index b4a966ab..2dd6e930 100644 --- a/config/config/src/sources/cli.rs +++ b/config/config/src/sources/cli.rs @@ -178,7 +178,7 @@ fn extend_cmd( .with_path(path.clone()), ) } - KeyGraph::Ref(_) => { + KeyGraph::Ref(_, _) => { return Err( ConfigError::new(ConfigErrorEnum::UnsupportedType(tree.key_type())) .with_path(path.clone()), @@ -227,7 +227,9 @@ pub fn generate_command() -> Result { command = command.help_template(template); - let map = match C::graph() { + let graph = C::graph(); + + let map = match graph.as_ref() { KeyGraph::Struct(map) => map, r => { return Err( @@ -491,7 +493,7 @@ fn matches_to_value( KeyGraph::Map(_, _) => { Err(ConfigError::new(ConfigErrorEnum::UnsupportedType(tree.key_type())).with_path(path)) } - KeyGraph::Ref(_) => { + KeyGraph::Ref(_, _) => { Err(ConfigError::new(ConfigErrorEnum::UnsupportedType(tree.key_type())).with_path(path)) } } @@ -508,7 +510,7 @@ impl CliSource { pub fn with_matches(matches: ArgMatches) -> Result { Ok(Self { - value: matches_to_value(KeyPath::new(), C::graph(), &matches, false) + value: matches_to_value(KeyPath::new(), &C::graph(), &matches, false) .map_err(|e| e.with_source(ErrorSource::Cli))? .unwrap_or(Value::Map(Default::default())), _phantom: PhantomData, diff --git a/config/config/src/sources/env.rs b/config/config/src/sources/env.rs index 231726fe..990d9819 100644 --- a/config/config/src/sources/env.rs +++ b/config/config/src/sources/env.rs @@ -25,7 +25,7 @@ impl EnvSource { Ok(Self { _phantom: PhantomData, value: extract_keys( - C::graph(), + &C::graph(), prefix, prefix .map(|p| KeyPath::new().push_child(p)) @@ -153,7 +153,7 @@ fn extract_keys( KeyGraph::Map(_, _) => { Err(ConfigError::new(ConfigErrorEnum::UnsupportedType(tree.key_type())).with_path(path)) } - KeyGraph::Ref(_) => { + KeyGraph::Ref(_, _) => { Err(ConfigError::new(ConfigErrorEnum::UnsupportedType(tree.key_type())).with_path(path)) } } diff --git a/config/config/src/tests.rs b/config/config/src/tests.rs index 067b5d49..60156660 100644 --- a/config/config/src/tests.rs +++ b/config/config/src/tests.rs @@ -2,7 +2,7 @@ //! This has to do with the way the macro generates code. It refers to items in the config crate with `::config` //! which is not available in the config crate itself. -use std::collections::BTreeMap; +use std::{collections::BTreeMap, sync::Arc}; use crate::{sources, Config, ConfigBuilder, Key, KeyGraph, Source, Value}; @@ -22,8 +22,9 @@ struct DummyConfig { // Can be generated with Config derive macro impl Config for DummyConfig { - fn graph() -> &'static KeyGraph { - if let Some(tree) = KeyGraph::get::() { + fn graph() -> Arc { + let builder = KeyGraph::builder::(); + if let Some(tree) = builder.get() { return tree; } @@ -32,7 +33,7 @@ impl Config for DummyConfig { keys.insert("enabled".to_string(), Key::new(bool::graph())); keys.insert("logging".to_string(), Key::new(LoggingConfig::graph())); - KeyGraph::store::(KeyGraph::Struct(keys)) + builder.build(KeyGraph::Struct(keys)) } } @@ -43,8 +44,9 @@ struct LoggingConfig { } impl Config for LoggingConfig { - fn graph() -> &'static KeyGraph { - if let Some(tree) = KeyGraph::get::() { + fn graph() -> Arc { + let builder = KeyGraph::builder::(); + if let Some(tree) = builder.get() { return tree; } @@ -53,7 +55,7 @@ impl Config for LoggingConfig { keys.insert("level".to_string(), Key::new(String::graph())); keys.insert("json".to_string(), Key::new(bool::graph())); - KeyGraph::store::(KeyGraph::Struct(keys)) + builder.build(KeyGraph::Struct(keys)) } } diff --git a/config/config_derive/src/lib.rs b/config/config_derive/src/lib.rs index 050139b9..38f54daa 100644 --- a/config/config_derive/src/lib.rs +++ b/config/config_derive/src/lib.rs @@ -113,8 +113,9 @@ fn impl_config(ast: &syn::DeriveInput) -> syn::Result { const VERSION: Option<&'static str> = option_env!("CARGO_PKG_VERSION"); const AUTHOR : Option<&'static str> = option_env!("CARGO_PKG_AUTHORS"); - fn graph() -> &'static ::config::KeyGraph { - if let Some(graph) = ::config::KeyGraph::get::() { + fn graph() -> ::std::sync::Arc<::config::KeyGraph> { + let builder = ::config::KeyGraph::builder::(); + if let Some(graph) = builder.get() { return graph; } @@ -122,7 +123,7 @@ fn impl_config(ast: &syn::DeriveInput) -> syn::Result { #(#keys_init)* - ::config::KeyGraph::store::(::config::KeyGraph::Struct(keys)) + builder.build(::config::KeyGraph::Struct(keys)) } } }