diff options
Diffstat (limited to 'tvix/eval/src/value')
| -rw-r--r-- | tvix/eval/src/value/json.rs | 8 | ||||
| -rw-r--r-- | tvix/eval/src/value/mod.rs | 4 | ||||
| -rw-r--r-- | tvix/eval/src/value/string.rs | 80 |
3 files changed, 64 insertions, 28 deletions
diff --git a/tvix/eval/src/value/json.rs b/tvix/eval/src/value/json.rs index c48e9c1f4e..24a6bcaf6f 100644 --- a/tvix/eval/src/value/json.rs +++ b/tvix/eval/src/value/json.rs @@ -47,8 +47,8 @@ impl Value { for val in l.into_iter() { match generators::request_to_json(co, val).await { - Ok((v, mut ctx)) => { - context = context.join(&mut ctx); + Ok((v, ctx)) => { + context.extend(ctx.into_iter()); out.push(v) } Err(cek) => return Ok(Err(cek)), @@ -100,8 +100,8 @@ impl Value { out.insert( name.to_str()?.to_owned(), match generators::request_to_json(co, value).await { - Ok((v, mut ctx)) => { - context = context.join(&mut ctx); + Ok((v, ctx)) => { + context.extend(ctx.into_iter()); v } Err(cek) => return Ok(Err(cek)), diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs index c171c9a04e..dfad0cd839 100644 --- a/tvix/eval/src/value/mod.rs +++ b/tvix/eval/src/value/mod.rs @@ -338,8 +338,8 @@ impl Value { let coerced: Result<BString, _> = match (value, kind) { // coercions that are always done (Value::String(mut s), _) => { - if let Some(ctx) = s.context_mut() { - context = context.join(ctx); + if let Some(ctx) = s.take_context() { + context.extend(ctx.into_iter()); } Ok((*s).into()) } diff --git a/tvix/eval/src/value/string.rs b/tvix/eval/src/value/string.rs index dd027895fd..4f869eb00d 100644 --- a/tvix/eval/src/value/string.rs +++ b/tvix/eval/src/value/string.rs @@ -54,6 +54,12 @@ impl From<HashSet<NixContextElement>> for NixContext { } } +impl<const N: usize> From<[NixContextElement; N]> for NixContext { + fn from(value: [NixContextElement; N]) -> Self { + Self(HashSet::from(value)) + } +} + impl NixContext { /// Creates an empty context that can be populated /// and passed to form a contextful [NixString], albeit @@ -78,20 +84,19 @@ impl NixContext { self } - /// Consumes both ends of the join into a new NixContent - /// containing the union of elements of both ends. - pub fn join(mut self, other: &mut NixContext) -> Self { - let other_set = std::mem::take(&mut other.0); - let mut set: HashSet<NixContextElement> = std::mem::take(&mut self.0); - set.extend(other_set); - Self(set) + /// Extends the existing context with more context elements. + pub fn extend<T>(&mut self, iter: T) + where + T: IntoIterator<Item = NixContextElement>, + { + self.0.extend(iter) } /// Copies from another [NixString] its context strings /// in this context. pub fn mimic(&mut self, other: &NixString) { if let Some(context) = other.context() { - self.0.extend(context.iter().cloned()); + self.extend(context.iter().cloned()); } } @@ -154,6 +159,16 @@ impl NixContext { } } +impl IntoIterator for NixContext { + type Item = NixContextElement; + + type IntoIter = std::collections::hash_set::IntoIter<NixContextElement>; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + /// This type is never instantiated, but serves to document the memory layout of the actual heap /// allocation for Nix strings. #[allow(dead_code)] @@ -530,11 +545,7 @@ impl<'a> From<&'a NixString> for &'a BStr { } } -impl From<NixString> for String { - fn from(s: NixString) -> Self { - s.to_string() - } -} +// No impl From<NixString> for String, that one quotes. impl From<NixString> for BString { fn from(s: NixString) -> Self { @@ -620,6 +631,11 @@ mod arbitrary { impl NixString { fn new(contents: &[u8], context: Option<Box<NixContext>>) -> Self { + debug_assert!( + !context.as_deref().is_some_and(NixContext::is_empty), + "BUG: initialized with empty context" + ); + // SAFETY: We're always fully initializing a NixString here: // // 1. NixStringInner::alloc sets up the len for us @@ -711,8 +727,10 @@ impl NixString { let context = [self.context(), other.context()] .into_iter() .flatten() - .fold(NixContext::new(), |acc_ctx, new_ctx| { - acc_ctx.join(&mut new_ctx.clone()) + .fold(NixContext::new(), |mut acc_ctx, new_ctx| { + // TODO: consume new_ctx? + acc_ctx.extend(new_ctx.iter().cloned()); + acc_ctx }); Self::new_context_from(context, s) } @@ -723,16 +741,30 @@ impl NixString { // // Also, we're using the same lifetime and mutability as self, to fit the // pointer-to-reference conversion rules - unsafe { NixStringInner::context_ref(self.0).as_deref() } + let context = unsafe { NixStringInner::context_ref(self.0).as_deref() }; + + debug_assert!( + !context.is_some_and(NixContext::is_empty), + "BUG: empty context" + ); + + context } - pub(crate) fn context_mut(&mut self) -> Option<&mut NixContext> { + pub(crate) fn context_mut(&mut self) -> &mut Option<Box<NixContext>> { // SAFETY: There's no way to construct an uninitialized or invalid NixString (see the SAFETY // comment in `new`). // // Also, we're using the same lifetime and mutability as self, to fit the // pointer-to-reference conversion rules - unsafe { NixStringInner::context_mut(self.0).as_deref_mut() } + let context = unsafe { NixStringInner::context_mut(self.0) }; + + debug_assert!( + !context.as_deref().is_some_and(NixContext::is_empty), + "BUG: empty context" + ); + + context } pub fn iter_context(&self) -> impl Iterator<Item = &NixContext> { @@ -760,12 +792,16 @@ impl NixString { self.context().is_some() } + /// This clears the context of the string, returning + /// the removed dependency tracking information. + pub fn take_context(&mut self) -> Option<Box<NixContext>> { + self.context_mut().take() + } + /// This clears the context of that string, losing /// all dependency tracking information. pub fn clear_context(&mut self) { - // SAFETY: There's no way to construct an uninitialized or invalid NixString (see the SAFETY - // comment in `new`). - *unsafe { NixStringInner::context_mut(self.0) } = None; + let _ = self.take_context(); } pub fn chars(&self) -> Chars<'_> { @@ -853,7 +889,7 @@ impl Display for NixString { } } -#[cfg(test)] +#[cfg(all(test, feature = "arbitrary"))] mod tests { use test_strategy::proptest; |