Skip to content

Fix field deduplication#129

Draft
inferiorhumanorgans wants to merge 2 commits intoembassy-rs:mainfrom
inferiorhumanorgans:fix-dedup-fields
Draft

Fix field deduplication#129
inferiorhumanorgans wants to merge 2 commits intoembassy-rs:mainfrom
inferiorhumanorgans:fix-dedup-fields

Conversation

@inferiorhumanorgans
Copy link
Copy Markdown
Contributor

#93 actually missed the part of the code where the field itself is named. This PR ensures fields are renamed.

This is marked as a draft because I added a couple tests to define the expected behavior. Beyond making sure that this is the right shape for tests to take, the duplicate_ir_fields_should_not_collide test fails. Should deduplication happen when parsing SVD and YAML definitions or just SVDs?

@datdenkikniet
Copy link
Copy Markdown
Contributor

datdenkikniet commented Apr 19, 2026

I think we should do something similar to #134: if we want field names to be unique in the IR, we should enforce that through its API. That way we'll know that having an IR means that there are no duplicate field names, and the boundary at which we are still "allowed" to have them (in the SVD, or in YAML files, or during a transform) is obvious and we can easily error out or do the transformation at that boundary.

If we don't do that, we should not do field deduplication in the IR at all, and we should rely on DeleteFields and AddFields transforms added by users instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants