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

@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?

//.impl_arbitrary(FeatureConfig::Gated("arbitrary")) // Optional impls.
//.impl_debug(FeatureConfig::Always) // See rustdoc for more,
//.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.

nyurik added 7 commits January 5, 2026 22:54
* 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`.
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
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