about summary refs log tree commit diff
diff options
context:
space:
mode:
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/attrs/mod.rs41
-rw-r--r--clippy_lints/src/attrs/repr_attributes.rs43
-rw-r--r--clippy_lints/src/declared_lints.rs1
-rw-r--r--clippy_utils/src/msrvs.rs1
-rw-r--r--tests/ui/default_union_representation.rs1
-rw-r--r--tests/ui/default_union_representation.stderr8
-rw-r--r--tests/ui/derive.rs1
-rw-r--r--tests/ui/derive.stderr20
-rw-r--r--tests/ui/repr_packed_without_abi.rs37
-rw-r--r--tests/ui/repr_packed_without_abi.stderr35
-rw-r--r--tests/ui/trailing_empty_array.rs1
-rw-r--r--tests/ui/trailing_empty_array.stderr22
13 files changed, 187 insertions, 25 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cc966972939..529ab6c20d6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5969,6 +5969,7 @@ Released 2018-09-13
 [`repeat_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_once
 [`repeat_vec_with_capacity`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_vec_with_capacity
 [`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
+[`repr_packed_without_abi`]: https://rust-lang.github.io/rust-clippy/master/index.html#repr_packed_without_abi
 [`reserve_after_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#reserve_after_initialization
 [`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs
 [`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used
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,
diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs
index f0381760709..35f552c15a3 100644
--- a/clippy_utils/src/msrvs.rs
+++ b/clippy_utils/src/msrvs.rs
@@ -24,6 +24,7 @@ msrv_aliases! {
     1,80,0 { BOX_INTO_ITER }
     1,77,0 { C_STR_LITERALS }
     1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
+    1,74,0 { REPR_RUST }
     1,73,0 { MANUAL_DIV_CEIL }
     1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }
     1,70,0 { OPTION_RESULT_IS_VARIANT_AND, BINARY_HEAP_RETAIN }
diff --git a/tests/ui/default_union_representation.rs b/tests/ui/default_union_representation.rs
index 41308b077ba..ba63cde2fa9 100644
--- a/tests/ui/default_union_representation.rs
+++ b/tests/ui/default_union_representation.rs
@@ -1,5 +1,6 @@
 #![feature(transparent_unions)]
 #![warn(clippy::default_union_representation)]
+#![allow(clippy::repr_packed_without_abi)]
 
 union NoAttribute {
     //~^ ERROR: this union has the default representation
diff --git a/tests/ui/default_union_representation.stderr b/tests/ui/default_union_representation.stderr
index c7ef70a0b8e..d558a3e8de1 100644
--- a/tests/ui/default_union_representation.stderr
+++ b/tests/ui/default_union_representation.stderr
@@ -1,5 +1,5 @@
 error: this union has the default representation
-  --> tests/ui/default_union_representation.rs:4:1
+  --> tests/ui/default_union_representation.rs:5:1
    |
 LL | / union NoAttribute {
 LL | |
@@ -13,7 +13,7 @@ LL | | }
    = help: to override `-D warnings` add `#[allow(clippy::default_union_representation)]`
 
 error: this union has the default representation
-  --> tests/ui/default_union_representation.rs:17:1
+  --> tests/ui/default_union_representation.rs:18:1
    |
 LL | / union ReprPacked {
 LL | |
@@ -25,7 +25,7 @@ LL | | }
    = help: consider annotating `ReprPacked` with `#[repr(C)]` to explicitly specify memory layout
 
 error: this union has the default representation
-  --> tests/ui/default_union_representation.rs:36:1
+  --> tests/ui/default_union_representation.rs:37:1
    |
 LL | / union ReprAlign {
 LL | |
@@ -37,7 +37,7 @@ LL | | }
    = help: consider annotating `ReprAlign` with `#[repr(C)]` to explicitly specify memory layout
 
 error: this union has the default representation
-  --> tests/ui/default_union_representation.rs:57:1
+  --> tests/ui/default_union_representation.rs:58:1
    |
 LL | / union ZSTAndTwoFields {
 LL | |
diff --git a/tests/ui/derive.rs b/tests/ui/derive.rs
index 3647b242505..dabc06bd7d1 100644
--- a/tests/ui/derive.rs
+++ b/tests/ui/derive.rs
@@ -2,6 +2,7 @@
     clippy::non_canonical_clone_impl,
     clippy::non_canonical_partial_ord_impl,
     clippy::needless_lifetimes,
+    clippy::repr_packed_without_abi,
     dead_code
 )]
 #![warn(clippy::expl_impl_clone_on_copy)]
diff --git a/tests/ui/derive.stderr b/tests/ui/derive.stderr
index c072a9a6277..0eb4b3c1ada 100644
--- a/tests/ui/derive.stderr
+++ b/tests/ui/derive.stderr
@@ -1,5 +1,5 @@
 error: you are implementing `Clone` explicitly on a `Copy` type
-  --> tests/ui/derive.rs:12:1
+  --> tests/ui/derive.rs:13:1
    |
 LL | / impl Clone for Qux {
 LL | |
@@ -10,7 +10,7 @@ LL | | }
    | |_^
    |
 note: consider deriving `Clone` or removing `Copy`
-  --> tests/ui/derive.rs:12:1
+  --> tests/ui/derive.rs:13:1
    |
 LL | / impl Clone for Qux {
 LL | |
@@ -23,7 +23,7 @@ LL | | }
    = help: to override `-D warnings` add `#[allow(clippy::expl_impl_clone_on_copy)]`
 
 error: you are implementing `Clone` explicitly on a `Copy` type
-  --> tests/ui/derive.rs:37:1
+  --> tests/ui/derive.rs:38:1
    |
 LL | / impl<'a> Clone for Lt<'a> {
 LL | |
@@ -34,7 +34,7 @@ LL | | }
    | |_^
    |
 note: consider deriving `Clone` or removing `Copy`
-  --> tests/ui/derive.rs:37:1
+  --> tests/ui/derive.rs:38:1
    |
 LL | / impl<'a> Clone for Lt<'a> {
 LL | |
@@ -45,7 +45,7 @@ LL | | }
    | |_^
 
 error: you are implementing `Clone` explicitly on a `Copy` type
-  --> tests/ui/derive.rs:49:1
+  --> tests/ui/derive.rs:50:1
    |
 LL | / impl Clone for BigArray {
 LL | |
@@ -56,7 +56,7 @@ LL | | }
    | |_^
    |
 note: consider deriving `Clone` or removing `Copy`
-  --> tests/ui/derive.rs:49:1
+  --> tests/ui/derive.rs:50:1
    |
 LL | / impl Clone for BigArray {
 LL | |
@@ -67,7 +67,7 @@ LL | | }
    | |_^
 
 error: you are implementing `Clone` explicitly on a `Copy` type
-  --> tests/ui/derive.rs:61:1
+  --> tests/ui/derive.rs:62:1
    |
 LL | / impl Clone for FnPtr {
 LL | |
@@ -78,7 +78,7 @@ LL | | }
    | |_^
    |
 note: consider deriving `Clone` or removing `Copy`
-  --> tests/ui/derive.rs:61:1
+  --> tests/ui/derive.rs:62:1
    |
 LL | / impl Clone for FnPtr {
 LL | |
@@ -89,7 +89,7 @@ LL | | }
    | |_^
 
 error: you are implementing `Clone` explicitly on a `Copy` type
-  --> tests/ui/derive.rs:82:1
+  --> tests/ui/derive.rs:83:1
    |
 LL | / impl<T: Clone> Clone for Generic2<T> {
 LL | |
@@ -100,7 +100,7 @@ LL | | }
    | |_^
    |
 note: consider deriving `Clone` or removing `Copy`
-  --> tests/ui/derive.rs:82:1
+  --> tests/ui/derive.rs:83:1
    |
 LL | / impl<T: Clone> Clone for Generic2<T> {
 LL | |
diff --git a/tests/ui/repr_packed_without_abi.rs b/tests/ui/repr_packed_without_abi.rs
new file mode 100644
index 00000000000..16b5ededee9
--- /dev/null
+++ b/tests/ui/repr_packed_without_abi.rs
@@ -0,0 +1,37 @@
+#![deny(clippy::repr_packed_without_abi)]
+
+#[repr(packed)]
+struct NetworkPacketHeader {
+    header_length: u8,
+    header_version: u16,
+}
+
+#[repr(packed)]
+union Foo {
+    a: u8,
+    b: u16,
+}
+
+#[repr(C, packed)]
+struct NoLintCNetworkPacketHeader {
+    header_length: u8,
+    header_version: u16,
+}
+
+#[repr(Rust, packed)]
+struct NoLintRustNetworkPacketHeader {
+    header_length: u8,
+    header_version: u16,
+}
+
+#[repr(packed, C)]
+union NotLintCFoo {
+    a: u8,
+    b: u16,
+}
+
+#[repr(packed, Rust)]
+union NotLintRustFoo {
+    a: u8,
+    b: u16,
+}
diff --git a/tests/ui/repr_packed_without_abi.stderr b/tests/ui/repr_packed_without_abi.stderr
new file mode 100644
index 00000000000..4f7acd00db3
--- /dev/null
+++ b/tests/ui/repr_packed_without_abi.stderr
@@ -0,0 +1,35 @@
+error: item uses `packed` representation without ABI-qualification
+  --> tests/ui/repr_packed_without_abi.rs:4:1
+   |
+LL |   #[repr(packed)]
+   |          ------ `packed` representation set here
+LL | / struct NetworkPacketHeader {
+LL | |     header_length: u8,
+LL | |     header_version: u16,
+LL | | }
+   | |_^
+   |
+   = warning: 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)]`
+note: the lint level is defined here
+  --> tests/ui/repr_packed_without_abi.rs:1:9
+   |
+LL | #![deny(clippy::repr_packed_without_abi)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: item uses `packed` representation without ABI-qualification
+  --> tests/ui/repr_packed_without_abi.rs:10:1
+   |
+LL |   #[repr(packed)]
+   |          ------ `packed` representation set here
+LL | / union Foo {
+LL | |     a: u8,
+LL | |     b: u16,
+LL | | }
+   | |_^
+   |
+   = warning: 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)]`
+
+error: aborting due to 2 previous errors
+
diff --git a/tests/ui/trailing_empty_array.rs b/tests/ui/trailing_empty_array.rs
index 3d06c262168..309a5920dfd 100644
--- a/tests/ui/trailing_empty_array.rs
+++ b/tests/ui/trailing_empty_array.rs
@@ -1,4 +1,5 @@
 #![warn(clippy::trailing_empty_array)]
+#![allow(clippy::repr_packed_without_abi)]
 
 // Do lint:
 
diff --git a/tests/ui/trailing_empty_array.stderr b/tests/ui/trailing_empty_array.stderr
index 756381478f2..7ebff372cf7 100644
--- a/tests/ui/trailing_empty_array.stderr
+++ b/tests/ui/trailing_empty_array.stderr
@@ -1,5 +1,5 @@
 error: trailing zero-sized array in a struct which is not marked with a `repr` attribute
-  --> tests/ui/trailing_empty_array.rs:5:1
+  --> tests/ui/trailing_empty_array.rs:6:1
    |
 LL | / struct RarelyUseful {
 LL | |
@@ -13,7 +13,7 @@ LL | | }
    = help: to override `-D warnings` add `#[allow(clippy::trailing_empty_array)]`
 
 error: trailing zero-sized array in a struct which is not marked with a `repr` attribute
-  --> tests/ui/trailing_empty_array.rs:11:1
+  --> tests/ui/trailing_empty_array.rs:12:1
    |
 LL | / struct OnlyField {
 LL | |
@@ -24,7 +24,7 @@ LL | | }
    = help: consider annotating `OnlyField` with `#[repr(C)]` or another `repr` attribute
 
 error: trailing zero-sized array in a struct which is not marked with a `repr` attribute
-  --> tests/ui/trailing_empty_array.rs:16:1
+  --> tests/ui/trailing_empty_array.rs:17:1
    |
 LL | / struct GenericArrayType<T> {
 LL | |
@@ -36,7 +36,7 @@ LL | | }
    = help: consider annotating `GenericArrayType` with `#[repr(C)]` or another `repr` attribute
 
 error: trailing zero-sized array in a struct which is not marked with a `repr` attribute
-  --> tests/ui/trailing_empty_array.rs:23:1
+  --> tests/ui/trailing_empty_array.rs:24:1
    |
 LL | / struct OnlyAnotherAttribute {
 LL | |
@@ -48,7 +48,7 @@ LL | | }
    = help: consider annotating `OnlyAnotherAttribute` with `#[repr(C)]` or another `repr` attribute
 
 error: trailing zero-sized array in a struct which is not marked with a `repr` attribute
-  --> tests/ui/trailing_empty_array.rs:30:1
+  --> tests/ui/trailing_empty_array.rs:31:1
    |
 LL | / struct OnlyADeriveAttribute {
 LL | |
@@ -60,7 +60,7 @@ LL | | }
    = help: consider annotating `OnlyADeriveAttribute` with `#[repr(C)]` or another `repr` attribute
 
 error: trailing zero-sized array in a struct which is not marked with a `repr` attribute
-  --> tests/ui/trailing_empty_array.rs:37:1
+  --> tests/ui/trailing_empty_array.rs:38:1
    |
 LL | / struct ZeroSizedWithConst {
 LL | |
@@ -72,7 +72,7 @@ LL | | }
    = help: consider annotating `ZeroSizedWithConst` with `#[repr(C)]` or another `repr` attribute
 
 error: trailing zero-sized array in a struct which is not marked with a `repr` attribute
-  --> tests/ui/trailing_empty_array.rs:47:1
+  --> tests/ui/trailing_empty_array.rs:48:1
    |
 LL | / struct ZeroSizedWithConstFunction {
 LL | |
@@ -84,7 +84,7 @@ LL | | }
    = help: consider annotating `ZeroSizedWithConstFunction` with `#[repr(C)]` or another `repr` attribute
 
 error: trailing zero-sized array in a struct which is not marked with a `repr` attribute
-  --> tests/ui/trailing_empty_array.rs:56:1
+  --> tests/ui/trailing_empty_array.rs:57:1
    |
 LL | / struct ZeroSizedWithConstFunction2 {
 LL | |
@@ -96,7 +96,7 @@ LL | | }
    = help: consider annotating `ZeroSizedWithConstFunction2` with `#[repr(C)]` or another `repr` attribute
 
 error: trailing zero-sized array in a struct which is not marked with a `repr` attribute
-  --> tests/ui/trailing_empty_array.rs:62:1
+  --> tests/ui/trailing_empty_array.rs:63:1
    |
 LL | struct ZeroSizedArrayWrapper([usize; 0]);
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -104,7 +104,7 @@ LL | struct ZeroSizedArrayWrapper([usize; 0]);
    = help: consider annotating `ZeroSizedArrayWrapper` with `#[repr(C)]` or another `repr` attribute
 
 error: trailing zero-sized array in a struct which is not marked with a `repr` attribute
-  --> tests/ui/trailing_empty_array.rs:65:1
+  --> tests/ui/trailing_empty_array.rs:66:1
    |
 LL | struct TupleStruct(i32, [usize; 0]);
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -112,7 +112,7 @@ LL | struct TupleStruct(i32, [usize; 0]);
    = help: consider annotating `TupleStruct` with `#[repr(C)]` or another `repr` attribute
 
 error: trailing zero-sized array in a struct which is not marked with a `repr` attribute
-  --> tests/ui/trailing_empty_array.rs:68:1
+  --> tests/ui/trailing_empty_array.rs:69:1
    |
 LL | / struct LotsOfFields {
 LL | |