Skip to content

Commit e530702

Browse files
fix(hydro_lang): remove SingletonRef global counter causing codegen non-determinism
The global `static SINGLETON_REF_COUNTER: AtomicUsize` was shared across all tests in a binary, causing `__hydro_singleton_ref_N` idents to shift depending on test execution order. For example, paxos snapshots expected IDs 0-6 but got 12-18 when singleton_ref tests ran first. Fix: Remove the global counter entirely. Instead, derive the ident index from `refs.len()` within each `with_singleton_capture` scope, so every closure's captured singleton refs are numbered starting from 0. This is safe because each closure gets its own lexical block in the generated code. Changes: - hydro_lang/src/singleton_ref.rs: Remove `SINGLETON_REF_COUNTER` static. Add `singleton_ref_ident(index)` helper. Simplify thread-local from `Vec<(syn::Ident, HydroNode)>` to `Vec<HydroNode>`. Use `refs.len()` as index. - hydro_lang/src/compile/ir/mod.rs: Change `ClosureExpr::singleton_refs` from `Vec<(syn::Ident, HydroNode)>` to `Vec<HydroNode>`. Update Clone, Serialize, deep_clone, transform_children, and emit_tokens to use index-based idents via the new helper. - hydro_test snapshots: Regenerated paxos IR and mermaid snapshots (stable only; nightly snapshots need a nightly CI run). Co-authored-by: Infinity 🤖 <infinity@hydro.run>
1 parent acaf9ed commit e530702

2 files changed

Lines changed: 39 additions & 36 deletions

File tree

hydro_lang/src/compile/ir/mod.rs

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ use backtrace::Backtrace;
4343
/// captures, which is important for nodes with multiple closures (e.g. Fold has `init` and `acc`).
4444
pub struct ClosureExpr {
4545
pub expr: DebugExpr,
46-
pub singleton_refs: Vec<(syn::Ident, HydroNode)>,
46+
pub singleton_refs: Vec<HydroNode>,
4747
}
4848

4949
impl Clone for ClosureExpr {
@@ -53,15 +53,12 @@ impl Clone for ClosureExpr {
5353
singleton_refs: self
5454
.singleton_refs
5555
.iter()
56-
.map(|(ident, node)| {
57-
let cloned_node = match node {
58-
HydroNode::Singleton { inner, metadata } => HydroNode::Singleton {
59-
inner: SharedNode(inner.0.clone()),
60-
metadata: metadata.clone(),
61-
},
62-
_ => panic!("singleton_refs should only contain HydroNode::Singleton"),
63-
};
64-
(ident.clone(), cloned_node)
56+
.map(|node| match node {
57+
HydroNode::Singleton { inner, metadata } => HydroNode::Singleton {
58+
inner: SharedNode(inner.0.clone()),
59+
metadata: metadata.clone(),
60+
},
61+
_ => panic!("singleton_refs should only contain HydroNode::Singleton"),
6562
})
6663
.collect(),
6764
}
@@ -90,14 +87,17 @@ impl serde::Serialize for ClosureExpr {
9087
}
9188
}
9289

93-
struct SerializableSingletonRefs<'a>(&'a [(syn::Ident, HydroNode)]);
90+
struct SerializableSingletonRefs<'a>(&'a [HydroNode]);
9491

9592
impl serde::Serialize for SerializableSingletonRefs<'_> {
9693
fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
9794
use serde::ser::SerializeSeq;
9895
let mut seq = serializer.serialize_seq(Some(self.0.len()))?;
99-
for (ident, node) in self.0 {
100-
seq.serialize_element(&(ident.to_string(), node))?;
96+
for (i, node) in self.0.iter().enumerate() {
97+
seq.serialize_element(&(
98+
crate::singleton_ref::singleton_ref_ident(i).to_string(),
99+
node,
100+
))?;
101101
}
102102
seq.end()
103103
}
@@ -134,7 +134,7 @@ impl From<DebugExpr> for ClosureExpr {
134134
}
135135

136136
impl ClosureExpr {
137-
pub fn new(expr: DebugExpr, singleton_refs: Vec<(syn::Ident, HydroNode)>) -> Self {
137+
pub fn new(expr: DebugExpr, singleton_refs: Vec<HydroNode>) -> Self {
138138
Self {
139139
expr,
140140
singleton_refs,
@@ -147,7 +147,7 @@ impl ClosureExpr {
147147
singleton_refs: self
148148
.singleton_refs
149149
.iter()
150-
.map(|(ident, node)| (ident.clone(), node.deep_clone(seen_tees)))
150+
.map(|node| node.deep_clone(seen_tees))
151151
.collect(),
152152
}
153153
}
@@ -157,7 +157,7 @@ impl ClosureExpr {
157157
transform: &mut impl FnMut(&mut HydroNode, &mut SeenSharedNodes),
158158
seen_tees: &mut SeenSharedNodes,
159159
) {
160-
for (_ident, ref_node) in self.singleton_refs.iter_mut() {
160+
for ref_node in self.singleton_refs.iter_mut() {
161161
transform(ref_node, seen_tees);
162162
}
163163
}
@@ -175,10 +175,8 @@ impl ClosureExpr {
175175
.into_iter()
176176
.rev()
177177
.collect::<Vec<_>>();
178-
let local_idents = self
179-
.singleton_refs
180-
.iter()
181-
.map(|(local_ident, _)| local_ident);
178+
let local_idents = (0..self.singleton_refs.len())
179+
.map(crate::singleton_ref::singleton_ref_ident);
182180
let hash = proc_macro2::Punct::new('#', proc_macro2::Spacing::Alone);
183181
let expr = &self.expr.0;
184182
quote! {

hydro_lang/src/singleton_ref.rs

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,18 @@ impl<T, L> Clone for SingletonRef<'_, '_, T, L> {
4141
}
4242

4343
// Thread-local storage for singleton references captured during `q!()` expansion.
44-
// Maps local ident name -> SharedNode for each singleton captured in the current closure.
44+
// Stores the HydroNode for each singleton captured in the current closure.
45+
// The index in the Vec determines the ident name via `singleton_ref_ident`.
4546
thread_local! {
46-
static SINGLETON_REFS: RefCell<Option<Vec<(syn::Ident, HydroNode)>>> = const { RefCell::new(None) };
47+
static SINGLETON_REFS: RefCell<Option<Vec<HydroNode>>> = const { RefCell::new(None) };
48+
}
49+
50+
/// Returns the canonical ident for a singleton ref at the given index within a closure.
51+
pub(crate) fn singleton_ref_ident(index: usize) -> syn::Ident {
52+
syn::Ident::new(
53+
&format!("__hydro_singleton_ref_{}", index),
54+
Span::call_site(),
55+
)
4756
}
4857

4958
/// Activate the singleton reference capture context. Must be called before `q!()` expansion
@@ -64,26 +73,23 @@ pub fn with_singleton_capture(
6473
crate::compile::ir::ClosureExpr::new(expr, singleton_refs)
6574
}
6675

67-
static SINGLETON_REF_COUNTER: std::sync::atomic::AtomicUsize =
68-
std::sync::atomic::AtomicUsize::new(0);
69-
7076
impl<'a, 'slf, T: 'a, L> FreeVariableWithContextWithProps<L, ()> for SingletonRef<'a, 'slf, T, L>
7177
where
7278
L: Location<'a>,
7379
{
7480
type O = &'a T;
7581

7682
fn to_tokens(self, _ctx: &L) -> (QuoteTokens, ()) {
77-
let id = SINGLETON_REF_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
78-
let ident = syn::Ident::new(&format!("__hydro_singleton_ref_{}", id), Span::call_site());
79-
80-
SINGLETON_REFS.with(|cell| {
83+
let ident = SINGLETON_REFS.with(|cell| {
8184
let mut guard = cell.borrow_mut();
8285
let refs = guard.as_mut().expect(
8386
"SingletonRef used inside q!() but no singleton capture scope is active. \
8487
This is a bug — singleton capture should be set up by the operator that uses q!().",
8588
);
8689

90+
let index = refs.len();
91+
let ident = singleton_ref_ident(index);
92+
8793
let metadata = self.ir_node.borrow().metadata().clone();
8894

8995
// Wrap in HydroNode::Singleton for materialization + identity tracking. If already a Singleton node,
@@ -101,13 +107,12 @@ where
101107
unreachable!()
102108
};
103109

104-
refs.push((
105-
ident.clone(),
106-
HydroNode::Singleton {
107-
inner: SharedNode(Rc::clone(&inner.0)),
108-
metadata,
109-
},
110-
));
110+
refs.push(HydroNode::Singleton {
111+
inner: SharedNode(Rc::clone(&inner.0)),
112+
metadata,
113+
});
114+
115+
ident
111116
});
112117

113118
(

0 commit comments

Comments
 (0)