Open
Conversation
Recognize `{static,const,build}_assert!` as special items, preferring
explicit `#[klint::diagnostic_item]` annotations and keeping a
conservative kernel-specific fallback for older trees.
The fallback resolves known macro paths structurally, extending the
existing diagnostic-item discovery pattern so later lints can identify
assert macros semantically.
Signed-off-by: Mohamad Alsadhan <mo@sdhn.cc>
Introduce the public surface for `build_assert_not_inlined`: - declare the lint - register the late lint pass - add derived diagnostics and the `inline(always)` suggestion helper - define the basic origin enum used by emitted notes This commit only adds the diagnostic shell and registration. The pass is present but does not perform semantic analysis yet. Signed-off-by: Mohamad Alsadhan <mo@sdhn.cc>
Add the cross-crate storage and lookup plumbing for `build_assert_not_inlined`. This introduces: - the semantic summary types used to describe whether a function's `build_assert!` behavior depends on runtime inputs - the `instance_build_assert_summary` persistent query, keyed by function instance - metadata table registration for storing those summaries alongside other `klint` query data - explicit metadata finalization/export hooks so stored summaries are written out at the end of analysis In other words, this commit adds the mechanism for recording and loading build_assert-related function summaries across crates. The query is still a stub in this commit. No analysis is performed yet; that comes in the following commit. Signed-off-by: Mohamad Alsadhan <mo@sdhn.cc>
Implement `build_assert_not_inlined` semantic analysis and lint emission. The analysis is MIR/instance-based and supports cross-crate summaries. It computes requirement summaries, propagates proven build_assert dependencies through relevant local functions, recovers local diagnostic origins, and emits the lint for functions that should be marked `#[inline(always)]`. This commit turns the previously added lint shell and summary plumbing into a working end-to-end lint. Signed-off-by: Mohamad Alsadhan <mo@sdhn.cc>
Add local UI coverage for `build_assert_not_inlined`. The tests cover direct build_assert use, helper propagation, recursive propagation, trait/default-method shapes, and negative cases such as unknown indirect calls that should not lint. Signed-off-by: Mohamad Alsadhan <mo@sdhn.cc>
Add a cross-crate regression test for `build_assert_not_inlined`. The test verifies that: - summaries are emitted by an upstream crate - downstream analysis loads those summaries - proven runtime-dependent cross-crate callers are linted - constant-only and unknown-indirect cases stay quiet Signed-off-by: Mohamad Alsadhan <mo@sdhn.cc>
Add docs for the new lint including the reasoning behind it, examples of when it triggers, and the expected `#[inline(always)]` remediation. Also add corresponding entry in README "Implemented Lints" section. Signed-off-by: Mohamad Alsadhan <mo@sdhn.cc>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR was split out from #13
Adds a
build_assert_not_inlinedLint:This is intended for
build_assert!uses whose error path must optimize awaye.g. cases where:
build_assert!const-only uses do not trigger the lint.Implementation Notes
This PR adds a dedicated
build_assert_not_inlinedanalysis/lint insrc/build_assert_not_inlined.rs.The lint:
build_assert!/build_error!build_assert!sites for diagnosticsThe implementation is not entirely complete for indirect calls; unsupported or unresolved indirect calls are treated as "unknown" rather than lint-worthy by default. This intentionally favors proven positives (too many false positives otherwise), but may miss some cases when indirect call targets cannot be resolved.
To avoid whole-crate MIR cost (too slow when I tried on the kernel), local analysis is seeded from relevant
build_assert!uses (in HIR) and propagated only through the necessary local slice.Other Changes / Refactors
src/diagnostic_items/out_of_band.rssame as Addassert_hierarchylint #14.Tests
Two separate test suites were added: normal UI compile-fail tests in
tests/ui/build_assert_not_inlined.rsand another test suite to verify "cross-crate" behaviour.tests/ui/build_assert_not_inlined.rsUI test coverage includes:
tests/build_assert_cross_crate.rscross-crate test coverage includes:
const-only and unknown-indirect negative casesI also tested it on the Linux kernel tree (Rust-for-Linux/linux@3a2486c), it did not trigger at all but when I manually removed
#[inline(always)], it was able to detect it.Docs
Add documentation in
doc/build_assert_not_inlined.mdand the corresponding README entry in the “Implemented Lints” section.Differences from the Older Approach in PR #13:
build_assert!calls, and conservative on unknown indirect callsLimitations
No support for cases like:
yet.
Open Questions
There are a few things I'm not entirely sure on and would appreciate some feedback or further discussion.
The current implementation uses a cheap local HIR prepass to identify functions that are plausibly relevant to
build_assert!, and only runs the heavier MIR/instance analysis on that subset. This avoids the whole-crate MIR cost that was intractable on the kernel codebase.Is this the right approach (does it miss anything?) or whether a different pruning/identification strategy is preferable?
Unsupported or unresolved indirect calls are treated as "unknown" rather than lint-worthy by default. In practice, this means the lint only fires for proven
build_assert!dependencies which sharply reduces false positives, but it also means some real cases may be missed when indirect call targets cannot be resolved.Should we prioritise high-confidence warnings or more conservative over-reporting, or is there an even better approach to deal with indirect calls?
Cross-crate summaries are keyed by
PseudoCanonicalInput<Instance<'tcx>>rather than plainDefId,because the relevant behavior often depends on the resolved downstream instance, not just the item definition. This gives us the precision needed, especially for generics, but it also makes the abstraction more MIR/monomorphization-oriented.Is
Instanceis the right long-term semantic unit, or whether a more abstract item-level summary model would be preferable later?Apologies, hope this was not too long.
Closes #7