dead_code false positive introduced in rustc 1.78.0-nightly (8ace7ea1f 2024-02-07)

[tamird]

#![deny(warnings)]

#[repr(u8)]
#[derive(Copy, Clone, Debug)]
pub enum RecordField {
    Target = 1,
    Level,
    Module,
    File,
    Line,
    NumArgs,
}

unsafe trait Pod {}

#[repr(transparent)]
struct RecordFieldWrapper(RecordField);

unsafe impl Pod for RecordFieldWrapper {}

fn try_read<T: Pod>(buf: &[u8]) -> T {
    unsafe { std::ptr::read_unaligned(buf.as_ptr() as *const T) }
}

pub fn foo(buf: &[u8]) -> RecordField {
    let RecordFieldWrapper(tag) = try_read(buf);
    tag
}

This code compiles on stable (and nightlies before 2024-02-07) but now produces:

error: struct `RecordFieldWrapper` is never constructed
  --> aya-log/src/lib.rs:79:8
   |
79 | struct RecordFieldWrapper(RecordField);
   |        ^^^^^^^^^^^^^^^^^^
   |
   = note: `RecordFieldWrapper` has a derived impl for the trait `Clone`, but this is intentionally ignored during dead code analysis
   = note: `-D dead-code` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(dead_code)]`

NOTE: playground's nightly is not new enough
playground

[apiraino]

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

[apiraino]

searched nightlies: from nightly-2024-02-01 to nightly-2024-02-08
regressed nightly: nightly-2024-02-08
searched commit range: 256b6fb...8ace7ea
regressed commit: d4f6f9e

bisected with cargo-bisect-rustc v0.6.7

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start=2024-02-01 --regress error --script script.sh 

cc @mu001999 @cjgillot

[Noratrieb]

How would you expect the compiler to figure out that it is constructed? Just because there's a value of that type somewhere doesn't mean it is, that would make the lint basically useless.

[mu001999]

By the way, the following code does not compile on stable (play):

#![deny(warnings)]

pub struct Bar(u32);

#[repr(transparent)]
struct Wrapper(Bar);

fn try_read<T>(p: &u32) -> T {
    unsafe { std::ptr::read_unaligned(p as *const u32 as *const T) }
}

pub fn foo(p: &u32) -> Bar {
    let Wrapper(bar) = try_read(p);
    bar
}

[Noratrieb]

Yeah, this isn't a regression, but a long standing issue (which seems hard to impossible to solve) that's now being exposed a bit more.
Would be nice if we could solve it, but I have doubts about that.

[oli-obk]

I don't see a way to solve this without looking through generic function calls (recursively, by looking at the function bodies). That also means it can only be done after monomorphization.

And even then it won't catch e.g. a transmute that produces a None. Instead that would be treated as producing a value of the Some type hypothetically.

Such deeper analyses maybe better suited to external tools that even do partial function instantiation, cross crate analysis...

[ryanavella]

At risk of mentioning the obvious, the example provided is not entirely minimal. It will work for any "constructor" of the form fn foo<T>() -> T, no trait bounds required.

I'm seeing this in some ffi-heavy code where it didn't trigger previously. In my case the "constructor" looks like this:

unsafe fn sysctl<T>(mib: &mut [c_int]) -> Option<T> {
    todo!()
}

[shepmaster]

It will work for any "constructor" of the form […] no trait bounds required.

Is that not what is shown in #120770 (comment) ?

[mu001999]

@rustbot claim

I suddenly realized that this probably didn't need to be done after monomorphization.

We won't meet this warning if specific the type:

#![deny(warnings)]

pub struct Bar(u32);

#[repr(transparent)]
struct Wrapper(Bar);

fn try_read<T>(p: &u32) -> T {
    unsafe { std::ptr::read_unaligned(p as *const u32 as *const T) }
}

pub fn foo(p: &u32) -> Bar {
    let Wrapper(bar) = try_read::<Wrapper>(p);
    bar
}

So I think maybe we can just mark the type live when visiting the corresponding pattern.

I will try it.

[compiler-errors]

I believe this is uncovered again since we reverted recent changes to dead code analysis

[hvenev-insait]

It can also trigger when the constructor is used in reachable code:

trait Constructible: Sized {
    fn new() -> Self;
}

struct My(i32);

impl Constructible for My {
    fn new() -> Self {
        My(42)
    }
}

fn make<M: Constructible>() -> M {
    M::new()
}

fn main() {
    let My(x) = make();
    eprintln!("ok {}", x);
}

Perhaps without monomorphization we could reason that:

1. The impl uses the constructor.
2. The impl itself isn't dead.
3. Therefore the constructor isn't dead.