From ab45ca57741d9a4d15b9bea1e9763033537911a5 Mon Sep 17 00:00:00 2001 From: datdenkikniet Date: Sat, 11 Apr 2026 13:36:36 +0200 Subject: [PATCH 1/6] check compatibillity when making field array --- src/transform/common.rs | 4 ++++ src/transform/make_field_array.rs | 23 +++++++++++++++++++++-- src/transform/make_register_array.rs | 4 ---- src/transform/merge_fieldsets.rs | 9 +++++++-- 4 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/transform/common.rs b/src/transform/common.rs index 379d1552..98174761 100644 --- a/src/transform/common.rs +++ b/src/transform/common.rs @@ -391,3 +391,7 @@ where pub(crate) fn get_true() -> bool { true } + +pub(crate) fn layout() -> CheckLevel { + CheckLevel::Layout +} diff --git a/src/transform/make_field_array.rs b/src/transform/make_field_array.rs index f36e0d31..c59dc600 100644 --- a/src/transform/make_field_array.rs +++ b/src/transform/make_field_array.rs @@ -1,9 +1,10 @@ -use anyhow::bail; +use anyhow::{bail, Context}; use log::*; use serde::{Deserialize, Serialize}; use super::common::*; use crate::ir::*; +use crate::transform::merge_fieldsets::field_compat; #[derive(Debug, Serialize, Deserialize)] pub struct MakeFieldArray { @@ -12,10 +13,14 @@ pub struct MakeFieldArray { pub to: String, #[serde(default)] pub mode: ArrayMode, + #[serde(default = "layout")] + pub check: CheckLevel, } impl MakeFieldArray { pub fn run(&self, ir: &mut IR) -> anyhow::Result<()> { + let mut errors = Vec::new(); + for id in match_all(ir.fieldsets.keys().cloned(), &self.fieldsets) { let b = ir.fieldsets.get_mut(&id).unwrap(); let groups = match_groups( @@ -32,7 +37,16 @@ impl MakeFieldArray { items.push(i); } - // todo check they're mergeable + let mut iter = items.iter(); + let main = iter.next().unwrap(); + + for other in iter { + errors.extend( + field_compat(main, other) + .into_iter() + .map(|v| (main.name.clone(), other.name.clone(), v)), + ); + } // one array shouldn't contain both regular and cursed bit_offset type { @@ -74,6 +88,11 @@ impl MakeFieldArray { b.fields.push(item); } } + + self.check + .check("making field arrays", &errors) + .context("failed to make field array")?; + Ok(()) } } diff --git a/src/transform/make_register_array.rs b/src/transform/make_register_array.rs index b63f164f..63b0294a 100644 --- a/src/transform/make_register_array.rs +++ b/src/transform/make_register_array.rs @@ -18,10 +18,6 @@ pub struct MakeRegisterArray { pub check: CheckLevel, } -fn layout() -> CheckLevel { - CheckLevel::Layout -} - impl MakeRegisterArray { pub fn run(&self, ir: &mut IR) -> anyhow::Result<()> { for id in match_all(ir.blocks.keys().cloned(), &self.blocks) { diff --git a/src/transform/merge_fieldsets.rs b/src/transform/merge_fieldsets.rs index 9be978c1..4fa0563b 100644 --- a/src/transform/merge_fieldsets.rs +++ b/src/transform/merge_fieldsets.rs @@ -270,19 +270,24 @@ impl core::fmt::Display for FieldError { } } +/// Check if two fields are compatible. +/// +/// bit offset is _not_ validated, as this technically does +/// not matter for compatibility. Callers of this function +/// are expected to validate bit offset correctness themselves +/// when necessary. pub(crate) fn field_compat(main: &Field, other: &Field) -> Vec { let mut errors = Vec::new(); let Field { name, description, - bit_offset, + bit_offset: _, bit_size, array, enumm, } = main; - assert_eq!(bit_offset, &other.bit_offset); assert_eq!(bit_size, &other.bit_size); match (array.as_ref(), other.array.as_ref()) { From f0fd68663569e93fad6059a2161889fa25f4f245 Mon Sep 17 00:00:00 2001 From: datdenkikniet Date: Sat, 11 Apr 2026 22:04:59 +0200 Subject: [PATCH 2/6] check compatibility when making blocks --- src/transform/make_block.rs | 33 ++++++++++++++++++++++++++------- src/transform/merge_blocks.rs | 30 ++++++++++++++++++++++-------- 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/src/transform/make_block.rs b/src/transform/make_block.rs index f3cc2018..47df8ebc 100644 --- a/src/transform/make_block.rs +++ b/src/transform/make_block.rs @@ -3,6 +3,7 @@ use serde::{Deserialize, Serialize}; use super::common::*; use crate::ir::*; +use crate::transform::merge_blocks::block_compat; #[derive(Debug, Serialize, Deserialize)] pub struct MakeBlock { @@ -13,26 +14,31 @@ pub struct MakeBlock { pub to_inner: String, #[serde(default)] pub array_on_outer: bool, + #[serde(default = "layout")] + pub check: CheckLevel, } impl MakeBlock { pub fn run(&self, ir: &mut IR) -> anyhow::Result<()> { + let mut had_breaking_error = false; + for id in match_all(ir.blocks.keys().cloned(), &self.blocks) { let b = ir.blocks.get_mut(&id).unwrap(); + + // Mapping of new item function to existing item functions let groups = match_groups( b.items.iter().map(|f| f.name.clone()), &self.from, &self.to_outer, ); + for (to, group) in groups { let b = ir.blocks.get_mut(&id).unwrap(); info!("blockifizing to {}", to); // Grab all items into a vec - let mut items = Vec::new(); - for i in b.items.iter().filter(|i| group.contains(&i.name)) { - items.push(i); - } + let mut items: Vec<_> = + b.items.iter().filter(|i| group.contains(&i.name)).collect(); // Sort by offs items.sort_by_key(|i| i.byte_offset); @@ -40,7 +46,6 @@ impl MakeBlock { info!(" {}", i.name); } - // todo check they're mergeable // todo check they're not arrays (arrays of arrays not supported) let byte_offset = items[0].byte_offset; @@ -63,9 +68,18 @@ impl MakeBlock { .collect(), }; - // TODO if destination block exists, check mergeable let dest = self.to_block.clone(); // todo regex - ir.blocks.insert(dest.clone(), b2); + if let Some(prev) = ir.blocks.insert(dest.clone(), b2.clone()) { + let errors: Vec<_> = block_compat(&prev, &b2) + .into_iter() + .map(|v| (dest.clone(), dest.clone(), v)) + .collect(); + + had_breaking_error |= self + .check + .check(&format!("making block {dest} from {group:?}"), &errors) + .is_err(); + } // Remove all items let b = ir.blocks.get_mut(&id).unwrap(); @@ -85,6 +99,11 @@ impl MakeBlock { }); } } + + if had_breaking_error { + anyhow::bail!("Failed to make block") + } + Ok(()) } } diff --git a/src/transform/merge_blocks.rs b/src/transform/merge_blocks.rs index 1a07c5da..bf205da4 100644 --- a/src/transform/merge_blocks.rs +++ b/src/transform/merge_blocks.rs @@ -75,11 +75,12 @@ fn merge_blocks( errors } -enum BlockError { +#[derive(Debug)] +pub(crate) enum BlockError { Description(Option, Option), Extends(Option, Option), - LhsMissingItem(&'static str, String), - RhsMissingItem(&'static str, String), + LhsMissingItem(&'static str, u32, String), + RhsMissingItem(&'static str, u32, String), Item(String, u32, BlockItemError), } @@ -101,13 +102,17 @@ impl core::fmt::Display for BlockError { BlockError::Item(name, offset, error) => { write!(f, "inner item {name} at offset {offset}: {error}") } - BlockError::LhsMissingItem(ty, i) => write!(f, "lhs is missing {ty} '{i}'"), - BlockError::RhsMissingItem(ty, i) => write!(f, "rhs is missing {ty} '{i}'"), + BlockError::LhsMissingItem(ty, offset, i) => { + write!(f, "lhs is missing {ty} '{i}' at offset {offset}") + } + BlockError::RhsMissingItem(ty, offset, i) => { + write!(f, "rhs is missing {ty} '{i}' at offset {offset}") + } } } } -fn block_compat(main: &Block, other: &Block) -> Vec { +pub(crate) fn block_compat(main: &Block, other: &Block) -> Vec { let mut errors = Vec::new(); let Block { @@ -138,7 +143,11 @@ fn block_compat(main: &Block, other: &Block) -> Vec { BlockItemInner::Register(_) => "register", }; - errors.push(BlockError::RhsMissingItem(ty, main.name.clone())); + errors.push(BlockError::RhsMissingItem( + ty, + main.byte_offset, + main.name.clone(), + )); continue; }; @@ -161,7 +170,11 @@ fn block_compat(main: &Block, other: &Block) -> Vec { BlockItemInner::Register(_) => "register", }; - errors.push(BlockError::LhsMissingItem(ty, other.name.clone())); + errors.push(BlockError::LhsMissingItem( + ty, + other.byte_offset, + other.name.clone(), + )); continue; }; } @@ -177,6 +190,7 @@ fn fmt_access(access: &Access) -> &str { } } +#[derive(Debug)] pub(crate) enum BlockItemError { Description(Option, Option), ArrayXor(bool, bool), From 7fcaab71b359122fb27b57d469754f5d74359716 Mon Sep 17 00:00:00 2001 From: datdenkikniet Date: Sat, 11 Apr 2026 22:12:25 +0200 Subject: [PATCH 3/6] collect all errors when making register array --- src/transform/make_register_array.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/transform/make_register_array.rs b/src/transform/make_register_array.rs index 63b0294a..3b05e0e9 100644 --- a/src/transform/make_register_array.rs +++ b/src/transform/make_register_array.rs @@ -20,6 +20,8 @@ pub struct MakeRegisterArray { impl MakeRegisterArray { pub fn run(&self, ir: &mut IR) -> anyhow::Result<()> { + let mut errors = Vec::new(); + for id in match_all(ir.blocks.keys().cloned(), &self.blocks) { let b = ir.blocks.get_mut(&id).unwrap(); let groups = match_groups(b.items.iter().map(|f| f.name.clone()), &self.from, &self.to); @@ -32,7 +34,6 @@ impl MakeRegisterArray { items.push(i); } - let mut errors = Vec::new(); let mut iter = items.iter(); let main = iter.next().unwrap(); @@ -44,10 +45,6 @@ impl MakeRegisterArray { ); } - self.check - .check("making register arrays", &errors) - .context("failed to make register array")?; - // todo check they're not arrays (arrays of arrays not supported) // Sort by offs @@ -71,6 +68,11 @@ impl MakeRegisterArray { b.items.push(item); } } + + self.check + .check("making/merging register arrays", &errors) + .context("failed to make register array")?; + Ok(()) } } From 034195ddfc540b0e9460cfe02b362d489bf1d6e0 Mon Sep 17 00:00:00 2001 From: datdenkikniet Date: Sun, 12 Apr 2026 22:21:55 +0200 Subject: [PATCH 4/6] Add deep validation --- src/transform/make_block.rs | 2 +- src/transform/make_register_array.rs | 6 ++- src/transform/merge_blocks.rs | 56 +++++++++++++++++----------- 3 files changed, 40 insertions(+), 24 deletions(-) diff --git a/src/transform/make_block.rs b/src/transform/make_block.rs index 47df8ebc..a0f0397b 100644 --- a/src/transform/make_block.rs +++ b/src/transform/make_block.rs @@ -70,7 +70,7 @@ impl MakeBlock { let dest = self.to_block.clone(); // todo regex if let Some(prev) = ir.blocks.insert(dest.clone(), b2.clone()) { - let errors: Vec<_> = block_compat(&prev, &b2) + let errors: Vec<_> = block_compat(ir, &prev, &b2) .into_iter() .map(|v| (dest.clone(), dest.clone(), v)) .collect(); diff --git a/src/transform/make_register_array.rs b/src/transform/make_register_array.rs index 3b05e0e9..f02fdeea 100644 --- a/src/transform/make_register_array.rs +++ b/src/transform/make_register_array.rs @@ -23,7 +23,7 @@ impl MakeRegisterArray { let mut errors = Vec::new(); for id in match_all(ir.blocks.keys().cloned(), &self.blocks) { - let b = ir.blocks.get_mut(&id).unwrap(); + let mut b = ir.blocks.get(&id).unwrap().clone(); let groups = match_groups(b.items.iter().map(|f| f.name.clone()), &self.from, &self.to); for (to, group) in groups { info!("arrayizing to {}", to); @@ -39,7 +39,7 @@ impl MakeRegisterArray { for other in iter { errors.extend( - block_item_compat(main, other) + block_item_compat(ir, main, other) .into_iter() .map(|v| (main.name.clone(), other.name.clone(), v)), ); @@ -67,6 +67,8 @@ impl MakeRegisterArray { item.byte_offset = offset; b.items.push(item); } + + ir.blocks.insert(id.clone(), b); } self.check diff --git a/src/transform/merge_blocks.rs b/src/transform/merge_blocks.rs index bf205da4..4c079a85 100644 --- a/src/transform/merge_blocks.rs +++ b/src/transform/merge_blocks.rs @@ -6,7 +6,7 @@ use std::fmt::{Display, Formatter}; use super::common::*; use crate::ir::*; -use crate::transform::merge_fieldsets::{array_compat, ArrayError}; +use crate::transform::merge_fieldsets::{array_compat, fieldset_compat, ArrayError, FieldSetError}; #[derive(Debug, Serialize, Deserialize)] pub struct MergeBlocks { @@ -60,7 +60,7 @@ fn merge_blocks( for id in &ids { let b2 = ir.blocks.get(id).unwrap(); errors.extend( - block_compat(&b, b2) + block_compat(ir, &b, b2) .into_iter() .map(|v| (main_id.clone(), id.clone(), v)), ); @@ -112,7 +112,7 @@ impl core::fmt::Display for BlockError { } } -pub(crate) fn block_compat(main: &Block, other: &Block) -> Vec { +pub(crate) fn block_compat(ir: &IR, main: &Block, other: &Block) -> Vec { let mut errors = Vec::new(); let Block { @@ -152,7 +152,7 @@ pub(crate) fn block_compat(main: &Block, other: &Block) -> Vec { }; errors.extend( - block_item_compat(main, other) + block_item_compat(ir, main, other) .into_iter() .map(|v| BlockError::Item(main.name.clone(), main.byte_offset, v)), ); @@ -199,8 +199,8 @@ pub(crate) enum BlockItemError { RegisterAccessMismatch(Access, Access), RegisterSizeMismatch(u32, u32), FieldSetXor(bool, bool), - FieldSet(String, String), - Block(String, String), + FieldSet(String, String, FieldSetError), + InnerBlock(String, String, Box), Name(String, String), } @@ -211,7 +211,8 @@ impl MinCheckLevel for BlockItemError { BlockItemError::Name(..) => CheckLevel::Names, BlockItemError::Array(_) => CheckLevel::Layout, BlockItemError::RegisterAccessMismatch(..) => CheckLevel::Layout, - BlockItemError::FieldSet(..) => CheckLevel::Layout, + BlockItemError::FieldSet(_, _, e) => e.min_check_level(), + BlockItemError::InnerBlock(_, _, e) => e.min_check_level(), _ => CheckLevel::Layout, } } @@ -249,8 +250,8 @@ impl Display for BlockItemError { FieldSetXor(false, _b) => { write!(f, "fieldset mismatch: lhs doesn't have fieldset, rhs does") } - FieldSet(l, r) => write!(f, "field set mismatch: {l} != {r}"), - Block(l, r) => write!(f, "inner block mismatch: {l} != {r}"), + FieldSet(l, r, e) => write!(f, "field set {l} and {r} mismatch: {e}"), + InnerBlock(l, r, e) => write!(f, "inner blocks {l} and {r} mismatch: {e}"), Name(l, r) => write!(f, "name mismatch: {l} != {r}"), } } @@ -262,7 +263,11 @@ impl Display for BlockItemError { /// not matter for compatibility. Callers of this function /// are expected to validate byte offset correctness themselves /// when necessary. -pub(crate) fn block_item_compat(main: &BlockItem, other: &BlockItem) -> Vec { +pub(crate) fn block_item_compat( + ir: &IR, + main: &BlockItem, + other: &BlockItem, +) -> Vec { let mut errors = Vec::new(); let BlockItem { @@ -294,13 +299,20 @@ pub(crate) fn block_item_compat(main: &BlockItem, other: &BlockItem) -> Vec { - if main != other { - errors.push(BlockItemError::Block( - main.block.clone(), - other.block.clone(), - )); - } + (Block(main_name), Block(other_name)) => { + let main = ir + .blocks + .get(&main_name.block) + .expect("main inner block exists"); + let other = ir.blocks.get(&other_name.block).expect("other name exists"); + + errors.extend(block_compat(ir, main, other).into_iter().map(|v| { + BlockItemError::InnerBlock( + main_name.block.clone(), + other_name.block.clone(), + Box::new(v), + ) + })); } (Register(main), Register(other)) => { let crate::ir::Register { @@ -324,10 +336,12 @@ pub(crate) fn block_item_compat(main: &BlockItem, other: &BlockItem) -> Vec { - if main != other { - errors.push(BlockItemError::FieldSet(main.clone(), other.clone())); - } + (Some(main_name), Some(other_name)) => { + let main = ir.fieldsets.get(main_name).expect("main fieldset exists"); + let other = ir.fieldsets.get(other_name).expect("other fieldset exists"); + errors.extend(fieldset_compat(main, other).into_iter().map(|v| { + BlockItemError::FieldSet(main_name.clone(), other_name.clone(), v) + })); } (None, None) => {} (main, other) => { From fd05df71fc74dc74254ae0af3d633bbc91792a2a Mon Sep 17 00:00:00 2001 From: datdenkikniet Date: Sun, 12 Apr 2026 22:29:16 +0200 Subject: [PATCH 5/6] Make this less spammy --- src/transform/common.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/transform/common.rs b/src/transform/common.rs index 98174761..b4071515 100644 --- a/src/transform/common.rs +++ b/src/transform/common.rs @@ -331,6 +331,8 @@ impl CheckLevel { had_breaking_error = true; } else if min_check_level == CheckLevel::Descriptions { log::debug!("{op} {main} and {other}: {error}"); + } else if min_check_level == CheckLevel::Names { + log::info!("{op} {main} and {other}: {error}"); } else { log::warn!("{op} {main} and {other}: {error}"); } From 0a78db6c99a43b2e65d67b5e1e161836969ef19a Mon Sep 17 00:00:00 2001 From: datdenkikniet Date: Sun, 12 Apr 2026 22:34:43 +0200 Subject: [PATCH 6/6] Track caller when logging errors --- src/transform/common.rs | 22 +++++++++++++++------- src/transform/make_block.rs | 6 +++++- src/transform/make_field_array.rs | 2 +- src/transform/make_register_array.rs | 2 +- src/transform/merge_blocks.rs | 2 +- src/transform/merge_fieldsets.rs | 2 +- 6 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/transform/common.rs b/src/transform/common.rs index b4071515..77e0db50 100644 --- a/src/transform/common.rs +++ b/src/transform/common.rs @@ -317,25 +317,33 @@ pub(crate) trait MinCheckLevel { } impl CheckLevel { - pub(crate) fn check(&self, op: &str, errors: &[(String, String, T)]) -> anyhow::Result<()> + pub(crate) fn check<'a, T>( + &self, + target: impl Into>, + op: &str, + errors: &[(String, String, T)], + ) -> anyhow::Result<()> where T: core::fmt::Display + MinCheckLevel, { let mut had_breaking_error = false; + let target = target.into().unwrap_or(module_path!()); for (main, other, error) in errors { let min_check_level = error.min_check_level(); - if self >= &min_check_level { - log::error!("{op} {main} and {other}: {error}"); + let level = if self >= &min_check_level { had_breaking_error = true; + log::Level::Error } else if min_check_level == CheckLevel::Descriptions { - log::debug!("{op} {main} and {other}: {error}"); + log::Level::Debug } else if min_check_level == CheckLevel::Names { - log::info!("{op} {main} and {other}: {error}"); + log::Level::Info } else { - log::warn!("{op} {main} and {other}: {error}"); - } + log::Level::Warn + }; + + log::log!(target: target, level, "{op} {main} and {other}: {error}"); } if had_breaking_error { diff --git a/src/transform/make_block.rs b/src/transform/make_block.rs index a0f0397b..d5bf0a5f 100644 --- a/src/transform/make_block.rs +++ b/src/transform/make_block.rs @@ -77,7 +77,11 @@ impl MakeBlock { had_breaking_error |= self .check - .check(&format!("making block {dest} from {group:?}"), &errors) + .check( + module_path!(), + &format!("making block {dest} from {group:?}"), + &errors, + ) .is_err(); } diff --git a/src/transform/make_field_array.rs b/src/transform/make_field_array.rs index c59dc600..8314e351 100644 --- a/src/transform/make_field_array.rs +++ b/src/transform/make_field_array.rs @@ -90,7 +90,7 @@ impl MakeFieldArray { } self.check - .check("making field arrays", &errors) + .check(module_path!(), "making field arrays", &errors) .context("failed to make field array")?; Ok(()) diff --git a/src/transform/make_register_array.rs b/src/transform/make_register_array.rs index f02fdeea..8b7dfbea 100644 --- a/src/transform/make_register_array.rs +++ b/src/transform/make_register_array.rs @@ -72,7 +72,7 @@ impl MakeRegisterArray { } self.check - .check("making/merging register arrays", &errors) + .check(module_path!(), "making/merging register arrays", &errors) .context("failed to make register array")?; Ok(()) diff --git a/src/transform/merge_blocks.rs b/src/transform/merge_blocks.rs index 4c079a85..0d9c1327 100644 --- a/src/transform/merge_blocks.rs +++ b/src/transform/merge_blocks.rs @@ -34,7 +34,7 @@ impl MergeBlocks { } self.check - .check("merging blocks", &errors) + .check(module_path!(), "merging blocks", &errors) .context("failed to merge blocks") } } diff --git a/src/transform/merge_fieldsets.rs b/src/transform/merge_fieldsets.rs index 4fa0563b..521986f1 100644 --- a/src/transform/merge_fieldsets.rs +++ b/src/transform/merge_fieldsets.rs @@ -32,7 +32,7 @@ impl MergeFieldsets { } self.check - .check("merging fieldsets", &errors) + .check(module_path!(), "merging fieldsets", &errors) .context("failed to merge fieldsets") } }