diff options
| author | Alex Macleod <alex@macleod.io> | 2024-12-16 02:17:59 +0000 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-12-16 02:17:59 +0000 |
| commit | 8da8da8428fcdff68d78e532b09e1b695a9de746 (patch) | |
| tree | 6aef59d8549a9400013048264ed7e19b0086a1a1 /clippy_lints/src | |
| parent | 60dbda209e9f3bf416f1dc4cd99d0d866f94b309 (diff) | |
| parent | 7a80f7b790333aed3b456db01d6c3dd26c69d487 (diff) | |
| download | rust-8da8da8428fcdff68d78e532b09e1b695a9de746.tar.gz rust-8da8da8428fcdff68d78e532b09e1b695a9de746.zip | |
Initial impl of `repr_packed_without_abi` (#13398)
Fixes #13375 I've added the lint next to the other attribute-related ones. Not sure if this is the correct place, since while we are looking after the `packed`-attribute (there is nothing we can do about types defined elsewhere), we are more concerned about the type's representation set by the attribute (instead of "duplicate attributes" and such). The lint simply looks at the attributes themselves without concern for the item-kind, since items where `repr` is not allowed end up in a compile-error anyway. I'm somewhat concerned about the level of noise this lint would cause if/when it goes into stable, although it does _not_ come up in `lintcheck`. ``` changelog: [`repr_packed_without_abi`]: Initial implementation ```
Diffstat (limited to 'clippy_lints/src')
| -rw-r--r-- | clippy_lints/src/attrs/mod.rs | 41 | ||||
| -rw-r--r-- | clippy_lints/src/attrs/repr_attributes.rs | 43 | ||||
| -rw-r--r-- | clippy_lints/src/declared_lints.rs | 1 |
3 files changed, 85 insertions, 0 deletions
diff --git a/clippy_lints/src/attrs/mod.rs b/clippy_lints/src/attrs/mod.rs index a9766597d50..92efd1a4ddc 100644 --- a/clippy_lints/src/attrs/mod.rs +++ b/clippy_lints/src/attrs/mod.rs @@ -7,6 +7,7 @@ mod duplicated_attributes; mod inline_always; mod mixed_attributes_style; mod non_minimal_cfg; +mod repr_attributes; mod should_panic_without_expect; mod unnecessary_clippy_cfg; mod useless_attribute; @@ -274,6 +275,44 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does + /// Checks for items with `#[repr(packed)]`-attribute without ABI qualification + /// + /// ### Why is this bad? + /// Without qualification, `repr(packed)` implies `repr(Rust)`. The Rust-ABI is inherently unstable. + /// While this is fine as long as the type is accessed correctly within Rust-code, most uses + /// of `#[repr(packed)]` involve FFI and/or data structures specified by network-protocols or + /// other external specifications. In such situations, the unstable Rust-ABI implied in + /// `#[repr(packed)]` may lead to future bugs should the Rust-ABI change. + /// + /// In case you are relying on a well defined and stable memory layout, qualify the type's + /// representation using the `C`-ABI. Otherwise, if the type in question is only ever + /// accessed from Rust-code according to Rust's rules, use the `Rust`-ABI explicitly. + /// + /// ### Example + /// ```no_run + /// #[repr(packed)] + /// struct NetworkPacketHeader { + /// header_length: u8, + /// header_version: u16 + /// } + /// ``` + /// + /// Use instead: + /// ```no_run + /// #[repr(C, packed)] + /// struct NetworkPacketHeader { + /// header_length: u8, + /// header_version: u16 + /// } + /// ``` + #[clippy::version = "1.84.0"] + pub REPR_PACKED_WITHOUT_ABI, + suspicious, + "ensures that `repr(packed)` always comes with a qualified ABI" +} + +declare_clippy_lint! { + /// ### What it does /// Checks for `any` and `all` combinators in `cfg` with only one condition. /// /// ### Why is this bad? @@ -415,6 +454,7 @@ pub struct Attributes { impl_lint_pass!(Attributes => [ INLINE_ALWAYS, + REPR_PACKED_WITHOUT_ABI, ]); impl Attributes { @@ -431,6 +471,7 @@ impl<'tcx> LateLintPass<'tcx> for Attributes { if is_relevant_item(cx, item) { inline_always::check(cx, item.span, item.ident.name, attrs); } + repr_attributes::check(cx, item.span, attrs, &self.msrv); } fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) { diff --git a/clippy_lints/src/attrs/repr_attributes.rs b/clippy_lints/src/attrs/repr_attributes.rs new file mode 100644 index 00000000000..e55a85b6267 --- /dev/null +++ b/clippy_lints/src/attrs/repr_attributes.rs @@ -0,0 +1,43 @@ +use rustc_ast::Attribute; +use rustc_lint::LateContext; +use rustc_span::{Span, sym}; + +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::msrvs; + +use super::REPR_PACKED_WITHOUT_ABI; + +pub(super) fn check(cx: &LateContext<'_>, item_span: Span, attrs: &[Attribute], msrv: &msrvs::Msrv) { + if msrv.meets(msrvs::REPR_RUST) { + check_packed(cx, item_span, attrs); + } +} + +fn check_packed(cx: &LateContext<'_>, item_span: Span, attrs: &[Attribute]) { + if let Some(items) = attrs.iter().find_map(|attr| { + if attr.ident().is_some_and(|ident| matches!(ident.name, sym::repr)) { + attr.meta_item_list() + } else { + None + } + }) && let Some(packed) = items + .iter() + .find(|item| item.ident().is_some_and(|ident| matches!(ident.name, sym::packed))) + && !items.iter().any(|item| { + item.ident() + .is_some_and(|ident| matches!(ident.name, sym::C | sym::Rust)) + }) + { + span_lint_and_then( + cx, + REPR_PACKED_WITHOUT_ABI, + item_span, + "item uses `packed` representation without ABI-qualification", + |diag| { + diag.warn("unqualified `#[repr(packed)]` defaults to `#[repr(Rust, packed)]`, which has no stable ABI") + .help("qualify the desired ABI explicity via `#[repr(C, packed)]` or `#[repr(Rust, packed)]`") + .span_label(packed.span(), "`packed` representation set here"); + }, + ); + } +} diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index c3501f6c5d7..7451fb909ef 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -55,6 +55,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::attrs::INLINE_ALWAYS_INFO, crate::attrs::MIXED_ATTRIBUTES_STYLE_INFO, crate::attrs::NON_MINIMAL_CFG_INFO, + crate::attrs::REPR_PACKED_WITHOUT_ABI_INFO, crate::attrs::SHOULD_PANIC_WITHOUT_EXPECT_INFO, crate::attrs::UNNECESSARY_CLIPPY_CFG_INFO, crate::attrs::USELESS_ATTRIBUTE_INFO, |
