Skip to content

Commit 48c3df4

Browse files
tamirmsclaude
andcommitted
Move wire size computation from parser to IR module
The wire size computation is only used by the IR builder, not by the parser or Rust code generator. Moving it to ir.rs: - Eliminates ComputedProperties/TypeProperties types (replaced by a plain HashMap internal to ir.rs) - Exposes a single ir::build() entry point that takes &XdrSpec and returns IR (hides TypeInfo, wire size computation, const resolution) - Collapses 13 lines of IR construction in main.rs to one call - Moves wire size tests from parser to generator (where the code lives) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 866811f commit 48c3df4

File tree

7 files changed

+251
-426
lines changed

7 files changed

+251
-426
lines changed

xdr-definitions-json/xdr.json

Lines changed: 0 additions & 124 deletions
Large diffs are not rendered by default.

xdr-generator-rust/generator/src/ir.rs

Lines changed: 137 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@
66
//! values) embedded directly.
77
88
use serde::Serialize;
9-
use std::collections::HashMap;
9+
use std::collections::{HashMap, HashSet};
1010
use xdr_parser::ast;
11+
use xdr_parser::types::TypeInfo;
1112

1213
/// Top-level IR output.
1314
#[derive(Serialize)]
@@ -147,12 +148,11 @@ pub enum TypeRef {
147148
},
148149
}
149150

150-
/// Build IR definitions from the AST spec and computed properties.
151-
pub fn build_definitions(
152-
spec: &ast::XdrSpec,
153-
computed: &xdr_parser::types::ComputedProperties,
154-
) -> Vec<Definition> {
155-
// Build enum member value lookup for resolving union case idents.
151+
/// Build the complete IR from a parsed (and optionally filtered) XDR spec.
152+
pub fn build(spec: &ast::XdrSpec, resolved_features: Vec<String>) -> IR {
153+
let type_info = TypeInfo::build(spec, &|name| name.to_string());
154+
let wire_sizes = compute_wire_sizes(&type_info);
155+
156156
let mut enum_member_values: HashMap<&str, i32> = HashMap::new();
157157
for def in spec.all_definitions() {
158158
if let ast::Definition::Enum(e) = def {
@@ -162,21 +162,30 @@ pub fn build_definitions(
162162
}
163163
}
164164

165-
// Build const value lookup for resolving named sizes.
166-
let const_values = &computed.const_values;
165+
let const_values = &type_info.const_values;
166+
167+
let definitions = spec.all_definitions()
168+
.map(|def| convert_definition(def, &wire_sizes, const_values, &enum_member_values))
169+
.collect();
167170

168-
spec.all_definitions()
169-
.map(|def| convert_definition(def, computed, const_values, &enum_member_values))
170-
.collect()
171+
IR {
172+
version: 1,
173+
files: spec.files.iter().map(|f| XdrFile {
174+
name: f.name.clone(),
175+
sha256: f.sha256.clone(),
176+
}).collect(),
177+
resolved_features,
178+
definitions,
179+
}
171180
}
172181

173182
fn convert_definition(
174183
def: &ast::Definition,
175-
computed: &xdr_parser::types::ComputedProperties,
184+
wire_sizes: &HashMap<String, Option<u32>>,
176185
const_values: &HashMap<String, i64>,
177186
enum_member_values: &HashMap<&str, i32>,
178187
) -> Definition {
179-
let fixed_size = computed.types.get(def.name()).and_then(|p| p.fixed_wire_size);
188+
let fixed_size = wire_sizes.get(def.name()).copied().flatten();
180189
match def {
181190
ast::Definition::Struct(s) => Definition::Struct(Struct {
182191
name: s.name.clone(),
@@ -293,3 +302,117 @@ fn resolve_size(size: &ast::Size, const_values: &HashMap<String, i64>) -> u64 {
293302
}
294303
}
295304
}
305+
306+
// =============================================================================
307+
// Wire size computation
308+
// =============================================================================
309+
310+
fn compute_wire_sizes(type_info: &TypeInfo) -> HashMap<String, Option<u32>> {
311+
let mut cache: HashMap<String, Option<u32>> = HashMap::new();
312+
let mut in_progress: HashSet<String> = HashSet::new();
313+
for name in type_info.definitions.keys() {
314+
compute_wire_size(type_info, name, &mut cache, &mut in_progress);
315+
}
316+
cache
317+
}
318+
319+
fn compute_wire_size(
320+
ti: &TypeInfo,
321+
name: &str,
322+
cache: &mut HashMap<String, Option<u32>>,
323+
in_progress: &mut HashSet<String>,
324+
) -> Option<u32> {
325+
if let Some(&cached) = cache.get(name) {
326+
return cached;
327+
}
328+
if in_progress.contains(name) {
329+
return None;
330+
}
331+
in_progress.insert(name.to_string());
332+
333+
let result = match ti.definitions.get(name) {
334+
Some(ast::Definition::Struct(s)) => {
335+
let mut total: u32 = 0;
336+
for member in &s.members {
337+
match type_wire_size(ti, &member.type_, cache, in_progress) {
338+
Some(sz) => {
339+
total = match total.checked_add(sz) {
340+
Some(t) => t,
341+
None => {
342+
in_progress.remove(name);
343+
cache.insert(name.to_string(), None);
344+
return None;
345+
}
346+
};
347+
}
348+
None => {
349+
in_progress.remove(name);
350+
cache.insert(name.to_string(), None);
351+
return None;
352+
}
353+
}
354+
}
355+
Some(total)
356+
}
357+
Some(ast::Definition::Union(u)) => {
358+
let disc_size: u32 = 4;
359+
let mut arm_sizes: Vec<Option<u32>> = Vec::new();
360+
for arm in &u.arms {
361+
match &arm.type_ {
362+
None => arm_sizes.push(Some(0)),
363+
Some(t) => arm_sizes.push(type_wire_size(ti, t, cache, in_progress)),
364+
}
365+
}
366+
if arm_sizes.is_empty()
367+
|| arm_sizes.iter().any(|s| s.is_none())
368+
|| arm_sizes.iter().filter_map(|s| *s).collect::<HashSet<_>>().len() != 1
369+
{
370+
None
371+
} else {
372+
disc_size.checked_add(arm_sizes[0]?)
373+
}
374+
}
375+
Some(ast::Definition::Enum(_)) => Some(4),
376+
Some(ast::Definition::Typedef(t)) => type_wire_size(ti, &t.type_, cache, in_progress),
377+
Some(ast::Definition::Const(_)) | None => None,
378+
};
379+
380+
in_progress.remove(name);
381+
cache.insert(name.to_string(), result);
382+
result
383+
}
384+
385+
fn type_wire_size(
386+
ti: &TypeInfo,
387+
type_: &ast::Type,
388+
cache: &mut HashMap<String, Option<u32>>,
389+
in_progress: &mut HashSet<String>,
390+
) -> Option<u32> {
391+
match type_ {
392+
ast::Type::Int | ast::Type::UnsignedInt | ast::Type::Bool | ast::Type::Float => Some(4),
393+
ast::Type::Hyper | ast::Type::UnsignedHyper | ast::Type::Double => Some(8),
394+
ast::Type::OpaqueFixed(size) => {
395+
let sz = resolve_size_u32(ti, size)?;
396+
sz.checked_add(3).map(|v| v & !3)
397+
}
398+
ast::Type::Array { element_type, size } => {
399+
let elem_sz = type_wire_size(ti, element_type, cache, in_progress)?;
400+
let count = resolve_size_u32(ti, size)?;
401+
elem_sz.checked_mul(count)
402+
}
403+
ast::Type::Ident(name) => compute_wire_size(ti, name, cache, in_progress),
404+
ast::Type::OpaqueVar(_) | ast::Type::String(_) | ast::Type::VarArray { .. } | ast::Type::Optional(_) => {
405+
None
406+
}
407+
}
408+
}
409+
410+
fn resolve_size_u32(ti: &TypeInfo, size: &ast::Size) -> Option<u32> {
411+
match size {
412+
ast::Size::Literal(n) => Some(*n),
413+
ast::Size::Named(name) => ti
414+
.const_values
415+
.get(name)
416+
.and_then(|&v| u32::try_from(v).ok()),
417+
}
418+
}

xdr-generator-rust/generator/src/main.rs

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -93,19 +93,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
9393
filter_spec(&mut ir_spec, &features);
9494
}
9595

96-
let type_info = xdr_parser::types::TypeInfo::build(&ir_spec, &|name| name.to_string());
97-
let computed = type_info.compute_properties();
98-
let definitions = ir::build_definitions(&ir_spec, &computed);
99-
100-
let output = ir::IR {
101-
version: 1,
102-
files: ir_spec.files.iter().map(|f| ir::XdrFile {
103-
name: f.name.clone(),
104-
sha256: f.sha256.clone(),
105-
}).collect(),
106-
resolved_features,
107-
definitions,
108-
};
96+
let output = ir::build(&ir_spec, resolved_features);
10997
let json = serde_json::to_string_pretty(&output)?;
11098
fs::write(json_path, json)?;
11199
eprintln!(

xdr-generator-rust/generator/src/tests/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@ mod cfg_filter;
22
mod generator;
33
mod naming;
44
mod types;
5+
mod wire_size;
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
use crate::ir;
2+
3+
fn wire_sizes(input: &str) -> std::collections::HashMap<String, Option<u32>> {
4+
let spec = xdr_parser::parser::parse(input).unwrap();
5+
let ir = ir::build(&spec, Vec::new());
6+
let mut sizes = std::collections::HashMap::new();
7+
for def in &ir.definitions {
8+
let (name, fixed_size) = match def {
9+
ir::Definition::Struct(s) => (&s.name, s.fixed_size),
10+
ir::Definition::Union(u) => (&u.name, u.fixed_size),
11+
_ => continue,
12+
};
13+
sizes.insert(name.clone(), fixed_size);
14+
}
15+
sizes
16+
}
17+
18+
#[test]
19+
fn test_fixed_wire_size() {
20+
let input = r#"
21+
struct Fixed { int x; unsigned hyper y; };
22+
struct Variable { int x; opaque data<>; };
23+
"#;
24+
let sizes = wire_sizes(input);
25+
assert_eq!(sizes["Fixed"], Some(12));
26+
assert_eq!(sizes["Variable"], None);
27+
}
28+
29+
#[test]
30+
fn test_union_fixed() {
31+
let input = r#"
32+
union U switch (int v) {
33+
case 0: int a;
34+
case 1: int b;
35+
};
36+
"#;
37+
let sizes = wire_sizes(input);
38+
assert_eq!(sizes["U"], Some(8));
39+
}
40+
41+
#[test]
42+
fn test_union_variable() {
43+
let input = r#"
44+
union U switch (int v) {
45+
case 0: int a;
46+
case 1: opaque b<>;
47+
};
48+
"#;
49+
let sizes = wire_sizes(input);
50+
assert_eq!(sizes["U"], None);
51+
}
52+
53+
#[test]
54+
fn test_union_different_arm_sizes() {
55+
let input = r#"
56+
union U switch (int v) {
57+
case 0: int a;
58+
case 1: hyper b;
59+
};
60+
"#;
61+
let sizes = wire_sizes(input);
62+
assert_eq!(sizes["U"], None);
63+
}
64+
65+
#[test]
66+
fn test_opaque_padding() {
67+
let input = "typedef opaque Hash[32];";
68+
let ir = ir::build(&xdr_parser::parser::parse(input).unwrap(), Vec::new());
69+
// Check via the TypeRef size on the typedef
70+
if let ir::Definition::Typedef(t) = &ir.definitions[0] {
71+
if let ir::TypeRef::OpaqueFixed { size } = &t.type_ {
72+
assert_eq!(*size, 32);
73+
} else {
74+
panic!("expected OpaqueFixed");
75+
}
76+
}
77+
}
78+
79+
#[test]
80+
fn test_opaque_padding_unaligned() {
81+
let input = "typedef opaque ShortHash[3];";
82+
let ir = ir::build(&xdr_parser::parser::parse(input).unwrap(), Vec::new());
83+
if let ir::Definition::Typedef(t) = &ir.definitions[0] {
84+
if let ir::TypeRef::OpaqueFixed { size } = &t.type_ {
85+
assert_eq!(*size, 3); // raw size, consumer computes padding
86+
} else {
87+
panic!("expected OpaqueFixed");
88+
}
89+
}
90+
}
91+
92+
#[test]
93+
fn test_fixed_array() {
94+
let input = r#"
95+
struct Pair { int a; int b; };
96+
struct Container { Pair items[3]; };
97+
"#;
98+
let sizes = wire_sizes(input);
99+
assert_eq!(sizes["Pair"], Some(8));
100+
assert_eq!(sizes["Container"], Some(24));
101+
}
102+
103+
#[test]
104+
fn test_typedef_chain() {
105+
let input = r#"
106+
typedef int MyInt;
107+
typedef MyInt AnotherInt;
108+
struct S { AnotherInt x; };
109+
"#;
110+
let sizes = wire_sizes(input);
111+
assert_eq!(sizes["S"], Some(4));
112+
}

0 commit comments

Comments
 (0)