Skip to content

fix: ast/message_id: use 0x1FFF_FFFF (1<<29 - 1) instead of 2^29 in test#77

Merged
nyurik merged 3 commits intooxibus:mainfrom
trnila:ci-fix-clippy-error
Mar 20, 2026
Merged

fix: ast/message_id: use 0x1FFF_FFFF (1<<29 - 1) instead of 2^29 in test#77
nyurik merged 3 commits intooxibus:mainfrom
trnila:ci-fix-clippy-error

Conversation

@trnila
Copy link
Copy Markdown
Member

@trnila trnila commented Mar 18, 2026

Clippy now emits the warning/error in CI:

error: using decimal literal for bitwise operation
  --> src/ast/message_id.rs:80:42
   |
80 |         let id = MessageId::Extended(2 ^ 29);
   |                                          ^^
   |
   = help: use binary (0b1_1101), hex (0x001d), or octal (0o35) notation for better readability
   = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.94.0/index.html#decimal_bitwise_operands
   = note: `-D clippy::decimal-bitwise-operands` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::decimal_bitwise_operands)]`

The test should rather test 0x1FFF_FFFF (1<<29 - 1) instead of 2 XOR 29 (31)

@trnila trnila requested a review from nyurik March 18, 2026 22:53
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@nyurik
Copy link
Copy Markdown
Member

nyurik commented Mar 18, 2026

this code in the original has always confused me - 2 << 29 makes no sense because it is the same as 1<<30 -- a much cleaner representation (unless there are a lot of different numbers all shifted by 29)... if you have any insight into this, please update the code to use 1 if it makes sense, and/or add some comments with explanation

@trnila trnila changed the title fix: ast/message_id: use 1<<29 instead of 2^29 in test fix: ast/message_id: use 0x1000_0000 (1<<28) instead of 2^29 in test Mar 19, 2026
@trnila trnila force-pushed the ci-fix-clippy-error branch from 493d3ba to 7f7ac28 Compare March 19, 2026 22:31
Clippy now emits the warning:

error: using decimal literal for bitwise operation
  --> src/ast/message_id.rs:80:42
   |
80 |         let id = MessageId::Extended(2 ^ 29);
   |                                          ^^
   |
   = help: use binary (0b1_1101), hex (0x001d), or octal (0o35) notation for better readability
   = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.94.0/index.html#decimal_bitwise_operands
   = note: `-D clippy::decimal-bitwise-operands` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::decimal_bitwise_operands)]`

The test should rather test 0x1FFF_FFFF (1<<29 - 1) instead of 2 XOR 29 (31)
@trnila trnila force-pushed the ci-fix-clippy-error branch from 7f7ac28 to 62c5439 Compare March 19, 2026 22:38
@trnila trnila changed the title fix: ast/message_id: use 0x1000_0000 (1<<28) instead of 2^29 in test fix: ast/message_id: use 0x1FFF_FFFF (1<<29 - 1) instead of 2^29 in test Mar 19, 2026
@trnila
Copy link
Copy Markdown
Member Author

trnila commented Mar 19, 2026

I have confused myself as well yesterday. I believe the original intent was to test the maximal extended id 0x1FFF_FFFF (1<<29 - 1) - so I have replaced it with the literal to make it more obvious.

Currently we dont check the validity of ID constructed via enum - maybe that would be good idea for both extended and standard ids?

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.

thx!

@nyurik nyurik enabled auto-merge (squash) March 20, 2026 05:18
@nyurik nyurik merged commit c681ff2 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