Warn on structs with a trailing zero-sized array that aren't repr(C)

[Manishearth]

struct Foo {
  /// ...
  last: [SomeType; 0]
}

such a struct is likely being created to pass to C (zero sized arrays aren't very useful in Rust itself) since the same pattern is sometimes used in C code for headers of allocations

credit @gankro for the idea

[Gankra]

may also just be used in conjuction with manual allocation to make it easy to compute the offset of the array, but the result is the same; need repr(C).

[Gankra]

a leading array could also reasonably get a warning to use repr(align) or whatever instead.

[kennytm]

You can't use #[repr(align)] if the align-as type doesn't have a fixed size (e.g. [usize; 0] or even [T; 0])

[brightly-salty]

I'm willing to attempt to implement this if it is still relevant.

[nhamovitz]

Hi o/ — first-time contributor here. I'm interested in trying to fix this. I've been reading some of the Clippy PRs lately so I have some idea where to start but I'll probably be back with questions 😅.

@rustbot claim

[nhamovitz]

Would this still apply if the zero-sized array was the only field in the struct?

[nhamovitz]

Also, would this have to be an early lint or a late lint? I'm going back and forth and chasing the types around is exhausting, but I feel like someone with more experience might just know. I guess specifically:

• Is the presence/absence of repr(C) still available in LateLintPass?
• Is the zero-sized-ness of the array available in EarlyLintPass?
• If "yes" to both, which would be preferable?

[xFrednet]

Hey @nhamovitz, regarding the implementation. The required information should all be available during both lint passes, it will probably be easier to use LateLintPass. Clippy also has more utility functions for it, and we have further information in general. Regarding the implementation, you might want to take a look at enum_clike_unportable_variant. That lint also checks for the repr attribute. The code can be found in clippy_lints/src/enum_clike.rs. 🙃

[nhamovitz]

What ought to be the behavior when a repr is specified, but repr(C) specifically is not, or where multiple reprs exist? Some examples:

#[repr(packed)]
struct ReprPacked {
    field: i32,
    last: [usize; 0],
}

#[repr(align(64))]
struct ReprAlign {
    field: i32,
    last: [usize; 0],
}

#[repr(packed, align(64))]
struct ReprPackedAlign {
    field: i32,
    last: [usize; 0],
}

#[repr(C, align(64))]
struct ReprCAlign {
    field: i32,
    last: [usize; 0],
}

(For reference, sym lists all these ways that something could be repr: repr, repr128, repr_align, repr_align_enum, repr_no_niche, repr_packed, repr_simd, repr_transparent.)

There's ReprOption::c, which AFAICT returns whether one of the indicated reprs is C, regardless of any others. Does that sound correct? Or should we say that if there's any repr attribute, then probably the user knows what they're doing?

[Manishearth]

I'd say any repr should suffice, most of them can be replicated in C

[nhamovitz]

Hi, another behavior question — what do you think about these?

#![feature(const_generics_defaults)]

struct ConstParamZeroDefault<const N: usize = 0> {
    last: [usize; N],
}

struct ConstParamNoDefault<const N: usize> {
    last: [usize; N],
}

struct ConstParamNonZeroDefault<const N: usize = 1> {
    last: [usize; N],
}

type A = ConstParamZeroDefault;
// etc

repr isn't valid on the type definitions, so we can't wait to see what N is for sure before deciding to lint. I'm inclined to say the first one should be linted + the others not; thoughts?

[Manishearth]

Nah none of these should be linted imo

[nhamovitz]

When linting struct definitions, is it convention to include any attributes on the definition in the pretty-printing? i.e.

error: trailing zero-sized array in a struct which is not marked with a `repr` attribute
  --> $DIR/trailing_zero_sized_array_without_repr.rs:20:1
   |
LL | / #[must_use]
LL | | struct OnlyAnotherAttribute {
LL | |     field: i32,
LL | |     last: [usize; 0],
LL | | }
   | |_^
   |
   = help: consider annotating the struct definition with `#[repr(C)]` or another `repr` attribute

or would we start at the struct line?

[nhamovitz]

OK I realized I could search for ItemKind::Struct and so I looked through and there weren't a lot of examples but based on exhaustive_items.rs and manual_non_exhaustive.rs it seems like the answer is no, don't include existing attributes in the lint pretty-print.

[Manishearth]

Correct!