Skip to content

Conversation

@nyurik
Copy link
Member

@nyurik nyurik commented Nov 27, 2025

  • Due to a compiler limitation, inner attributes are not allowed with the include! macro. This PR removes all #![...] and //! ... from the generated code, and adds many explicit allow to individual functions and use statements where relevant.
mod messages {
    include!(concat!(env!("OUT_DIR"), "/messages.rs"));
}
  • Do not place generated code inside the user's code - not a good practice. All generated code should go into OUT_DIR.
  • Remove allow_dead_code configuration
  • Consolidate all documentation in the README, and use it as docs
  • Minor Cargo.toml cleanup

@nyurik nyurik changed the title chore: allow generated file to be included chore: allow generated file to be used with include! Nov 27, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the code generation to make generated files compatible with Rust's include! macro by removing inner attributes (#![...] and //! ...), which are not allowed due to a compiler limitation. Generated code is now written to OUT_DIR during the build process and included via include! in user code, following Rust best practices for build-time code generation.

Key changes:

  • Generated code now omits all inner attributes and doc comments, moving lint suppressions to module-level #[expect(...)] attributes
  • All generated code is written to OUT_DIR instead of the source tree
  • Removed the allow_dead_code configuration option
  • Updated error trait implementation from std::error::Error to core::error::Error for better no_std compatibility

Reviewed changes

Copilot reviewed 11 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
testing/can-messages/src/messages.rs Entire generated file removed from source tree
testing/can-messages/src/lib.rs Replaced static module with include! from OUT_DIR, added module-level lint suppressions
testing/can-messages/build.rs Simplified to write generated code directly to OUT_DIR instead of src/
testing/can-embedded/src/messages.rs Stub file removed as it's now generated during build
testing/can-embedded/src/lib.rs Added include! macro to load generated code from OUT_DIR
testing/can-embedded/build.rs New build script to generate messages.rs for embedded target
testing/can-embedded/Cargo.toml Added build dependencies for code generation
src/lib.rs Removed inner attributes and doc comments from generated code, changed error trait to core::error::Error, removed allow_dead_code config
README.md Removed reference to allow_dead_code configuration option
Cargo.lock Dependency version updates
.github/workflows/ci.yml Updated cargo commands to use --workspace instead of --all

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nyurik nyurik force-pushed the rm-messages branch 4 times, most recently from 61b4663 to 2a186df Compare November 28, 2025 22:38
Copy link
Member

@tegimeki tegimeki left a comment

Choose a reason for hiding this comment

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

These changes look good to me and address issues I've considered important, namely having generated code in OUT_DIR rather than within the source tree. However, not being a user of this crate currently it seems best to ping some of the recent committers to get their take: @killercup @scootermon @projectgus @trnila @cpctaylo

This seems like a breaking change, not necessarily at the API level but in terms of how dependent projects might use the generated files... what kind of version bump is anticipated?

@@ -50,23 +50,36 @@ fn main() {
//.check_ranges(FeatureConfig::Never) // or look below for an example.
.build();
Copy link
Member

Choose a reason for hiding this comment

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

can we add imports to have working example in README?

use dbc_codegen::Config;
use std::fs::write;
use std::{env::var, path::PathBuf};

dbc_file should be as a string now:

let dbc_file = std::fs::read_to_string(dbc_path).unwrap();

Copy link
Member Author

Choose a reason for hiding this comment

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

@trnila I made README to be the only documentation for the crate, and for it to be auto-compiled as part of the doc tests.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 17 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nyurik nyurik requested review from tegimeki and trnila January 6, 2026 06:22
@nyurik
Copy link
Member Author

nyurik commented Jan 9, 2026

@trnila are you able to compile everything with the new changes? Anything else blocking this? thx!

@trnila
Copy link
Member

trnila commented Jan 10, 2026

@nyurik Just having this last issue in my app:

warning: variant `InvalidMultiplexor` is never constructed
   --> out/messages.rs:469:5
    |
459 | pub enum CanError {
    |          -------- variant in this enum
...
469 |     InvalidMultiplexor {
    = note: `CanError` has derived impls for the traits `Debug` and `Clone`, but these are intentionally ignored during dead code analysis
    = note: `#[warn(dead_code)]` (part of `#[warn(unused)]`) on by default

Can be reproduced by using the same minimalistic dbc testing/dbc-examples/example.dbc:

BO_ 256 msg1: 1 tx
 SG_ test : 0|8@1+ (1,0) [0|255] ""  rx

and commenting out pub use messages::*; in testing/can-messages/src/lib.rs.
This line exports everything including CanError and somebody else in different location could construct InvalidMultiplexor.

We are using dbc-codegen directly in our binary without any libs, so rust detects this dead code correctly...

* Due to a compiler limitation, inner attributes are not allowed with the `include!` macro.  This PR removes all `#![...]` and `//! ...` from the generated code, which will result in some warnings being generated. We may need to add explicit `allow` to each individual function where relevant.
* Do not place generated code inside the user's code - not a good practice.  All generated code should go into `OUT_DIR`.
@nyurik
Copy link
Member Author

nyurik commented Jan 10, 2026

@trnila thx, fixed

@nyurik nyurik merged commit 1c4dfb5 into oxibus:main Jan 10, 2026
4 checks passed
@nyurik nyurik deleted the rm-messages branch January 10, 2026 22:24
@nyurik nyurik mentioned this pull request Jan 20, 2026
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.

3 participants