fix(build): Meaningful compilation errors when no backend feature enabled #5767
Conversation
Currently, when no backend is enabled, we get a horrible mess of errors. This commit: * Makes sure the wasmer-compiler crate can be compiled withou the compiler feature enabled * Makes sure the compiler crate fails to compile with a single meaningful message if neither core nor std features are enabled * Makes sure the api crate fails to compile with a single meaninful error when no backend is enabled Note: our current module structure doesn't make it trivial to do this cleanly. I went for an approach that has more duplication, but is minimally invasive.
|
LGTM, but can we turn the overly verbose |
theduke
changed the title
Done that. @theduke please check my change hasn't changed the expected build behavior? |
There was a problem hiding this comment.
Pull request overview
This PR improves compilation error messages when required backend features are not enabled. Currently, missing features result in cascading compilation errors that are difficult to debug. The changes introduce meaningful compile_error! messages and feature-gate code appropriately.
- Adds compile_error! macros to catch missing backend/compiler feature combinations early
- Introduces cfg_std_or_core! and cfg_compiler! macros to reduce duplication in conditional compilation
- Adds feature = "compiler" guards to perfmap-related functions that depend on compiler infrastructure
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/compiler/src/lib.rs | Introduces cfg_std_or_core! macro and wraps module declarations to ensure they're only compiled when std or core features are enabled |
| lib/compiler/src/engine/inner.rs | Adds feature = "compiler" guard to register_perfmap function which accesses compiler-specific fields |
| lib/compiler/src/engine/artifact.rs | Adds feature = "compiler" guard to register_perfmap call site for consistency |
| lib/api/src/lib.rs | Adds compile_error! checks for missing backend features, introduces cfg_compiler! macro to conditionally compile modules based on enabled backends |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| pub use entities::memory::{MemoryView, location::MemoryLocation}; | ||
| mod error; | ||
| pub use error::*; | ||
| pub use entities::*; |
There was a problem hiding this comment.
[nitpick] The cfg_compiler! macro block (lines 455-465) includes re-exports from the entities module (lines 458, 461), but the actual mod entities; declaration is located after the macro block (line 478). While both are guarded by the same feature flags, this separation makes the code harder to follow because:
- The re-exports appear before the module declaration
- The dependency on
entitiesis not immediately visible within the macro block - A reader might not realize that
entitiesmust be declared elsewhere
Consider adding a comment within the cfg_compiler! block before the entities re-exports (e.g., at line 458) noting that mod entities; is declared separately below due to the ambiguity issue mentioned in the TODO. This would help readers understand the relationship without needing to search through the file.
| mod backend; | ||
| pub use backend::*; | ||
| mod vm; | ||
| // TODO: cannot be placed into cfg_compiler due to: error: `inner` is ambiguous |
There was a problem hiding this comment.
[nitpick] The TODO comment states that mod entities; cannot be placed into cfg_compiler! due to an "inner is ambiguous" error, but doesn't provide details about what causes this ambiguity or how it might be resolved in the future.
Consider expanding this comment to explain:
- What specific ambiguity error occurs when placing
entitiesin the macro - What "inner" refers to (likely the
inner.rsfiles in entity modules) - Whether this is a temporary workaround or if there's a plan to restructure the module layout to avoid this issue
This will help future maintainers understand the constraint and potentially address it when refactoring.
| // TODO: cannot be placed into cfg_compiler due to: error: `inner` is ambiguous | |
| // NOTE: `mod entities;` cannot be placed inside `cfg_compiler!` due to a Rust compiler ambiguity error: | |
| // When `entities` is included in the macro, the compiler reports "`inner` is ambiguous" because multiple | |
| // submodules within `entities` (e.g., `entities/memory/inner.rs`, `entities/table/inner.rs`, etc.) have files named `inner.rs`. | |
| // This causes the module resolution to fail, as Rust cannot disambiguate which `inner` is being referred to. | |
| // As a workaround, `mod entities;` is declared outside the macro with explicit `#[cfg]` gating. | |
| // If the module layout is restructured in the future (e.g., by renaming `inner.rs` files or flattening the hierarchy), | |
| // this workaround may no longer be necessary and `mod entities;` could be moved into `cfg_compiler!`. |
Currently, when no backend is enabled, we get a horrible mess of errors.
This commit:
compiler feature enabled
meaningful message if neither core nor std features are enabled
error when no backend is enabled
Note: our current module structure doesn't make it trivial to do this
cleanly.
I went for an approach that has more duplication, but is minimally
invasive.