Skip to content

fix: parse signal min/max as NumericValue#76

Merged
nyurik merged 4 commits intooxibus:mainfrom
trnila:minmax_numeric_value
Mar 20, 2026
Merged

fix: parse signal min/max as NumericValue#76
nyurik merged 4 commits intooxibus:mainfrom
trnila:minmax_numeric_value

Conversation

@trnila
Copy link
Copy Markdown
Member

@trnila trnila commented Feb 24, 2026

Min/max values like 18446744073709551615 cannot be represented as f64 without the loss of precision - they are represented as 18446744073709552000 instead.
Use NumericValue to parse the value as i64, u64 or f64, but its API breaking change.

This will fix uncompilable test case in dbc_codegen.

Not sure if we should enforce both min and max are the same NumericValue enum variant or create a new Limit type? But this is maybe the responsibility of dbc-codegen?

@trnila trnila force-pushed the minmax_numeric_value branch 2 times, most recently from 7113604 to af80d9f Compare February 24, 2026 21:00
@trnila trnila requested a review from nyurik February 24, 2026 21:04
min:
Uint: 0
max:
Uint: 18446744073709551615
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix for failing compilation on dbc-codegen.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 24, 2026

Codecov Report

❌ Patch coverage is 93.75000% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/parser.rs 42.85% 0 Missing and 4 partials ⚠️
src/ast/signal.rs 80.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@trnila trnila force-pushed the minmax_numeric_value branch 2 times, most recently from c8dd3a7 to ba5bc06 Compare March 18, 2026 22:33
Min/max values like 18446744073709551615 cannot be represented as f64
without the loss of precision. Use NumericValue to parse the value as
i64, u64 or f64.
@trnila trnila force-pushed the minmax_numeric_value branch 2 times, most recently from 5f475f0 to 3a07492 Compare March 18, 2026 22:53
@nyurik
Copy link
Copy Markdown
Member

nyurik commented Mar 18, 2026

i'm ok with the breaking change if it makes ecosystem better - plus its not that significant it seems...

Copy link
Copy Markdown
Member

@nyurik nyurik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@nyurik nyurik merged commit 3cafcb1 into oxibus:main Mar 20, 2026
9 checks passed
@nyurik nyurik mentioned this pull request Mar 20, 2026
nyurik added a commit that referenced this pull request Mar 20, 2026
## 🤖 New release

* `can-dbc`: 8.1.0 -> 9.0.0 (⚠ API breaking changes)

### ⚠ `can-dbc` breaking changes

```text
--- failure copy_impl_added: type now implements Copy ---

Description:
A public type now implements Copy, causing non-move closures to capture it by reference instead of moving it.
        ref: rust-lang/rust#100905
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/copy_impl_added.ron

Failed in:
  can_dbc::NumericValue in /tmp/.tmpiaRx6y/can-dbc/src/ast/numeric_value.rs:7
```

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## [9.0.0](v8.1.0...v9.0.0) -
2026-03-20

### Fixed

- parse signal min/max as `NumericValue`
([#76](#76))
- ast/message_id: use 0x1FFF_FFFF (1<<29 - 1) instead of 2^29 in test
([#77](#77))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).
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