impls should not inhibit the unused_item lint

[nagisa]

#![crate_type="lib"]

enum Foo {
}

impl Clone for Foo {
    fn clone(&self) -> Foo { loop {} }
}

This code emits no warnings because impl Clone for Foo is considered a "use" of the enum, however, implementation for this enum cannot possibly be used either, as the enum is private and is unused. This is more notable for derived implementations, which also inhibit this lint.

The inherent implementations behave as expected.

[shepmaster]

Does anyone have any suggestions on what part of the code to look at to attempt to fix this?

[SimonSapin]

The warning message is:

enum Foo {}

(Playground)

Errors:

   Compiling playground v0.0.1 (/playground)
warning: enum is never used: `Foo`
 --> src/lib.rs:1:6
  |
1 | enum Foo {}
  |      ^^^
  |
  = note: `#[warn(dead_code)]` on by default

---

None of the results of rg 'is never used' src/lib* seem relevant, but looking for is never finds:

rust/src/librustc_passes/dead.rs

Lines 1 to 3 in 31add7e

	// This implements the dead-code warning pass. It follows middle::reachable
	// closely. The idea is that all reachable symbols are live, codes called
	// from live codes are live, and everything else is dead.

rust/src/librustc_passes/dead.rs

Line 561 in 31add7e

lint.build(&format!("{} is never {}: `{}`", descr, participle, name)).emit()

Presumably the current implementation of this warning unconditionally considers code in trait impls to be reachable. I assume that privacy is also involved: a "normal" function is dead code if it’s private and never called, but if it’s public and never called in this crate it could still be called by a downstream dependency so it has to be considered reachable. However trait impls do not have privacy of their own, so it’s not as easy to find out if there could be a caller in another crate.

I believe the change needed here is, instead of always treating trait impls as "public", use the most restrictive privacy between that of the trait and that of the type. This can be tricky when the impl is generic and may require to be conservative in some cases, but I believe it’s still possible to do better than the status quo.

[shepmaster]

I think that this specifically comes from when Self is used in a trait:

struct Foo;

// Does not prevent the warning
trait Bar0 {}
impl Bar0 for Foo {}

// Does not prevent the warning
trait Bar1 { fn dummy(); }
impl Bar1 for Foo { fn dummy () {} }

// Prevents the warning
trait Bar2 { fn dummy(&self); }
impl Bar2 for Foo { fn dummy (&self) {} }

// Prevents the warning
trait Bar3 { fn dummy() -> Self; }
impl Bar3 for Foo { fn dummy () -> Self { todo!() } }

// Prevents the warning
trait Bar4 { type Dummy; }
impl Bar4 for Foo { type Dummy = Self; }

// Prevents the warning
trait Bar5 { const DUMMY: Self; }
impl Bar5 for Foo { const DUMMY: Self = Self; }

[LDVSOFT]

Any updates here?

[kpreid]

Would it be reasonable to add a special signal to Clone's expansion which doesn't count it as a construction of the struct? Precedent is the way that Debug now does not cause struct fields to count as read for dead code analysis.

(I just hit this bug today, and it briefly gave me worries of “is my file not being compiled? I should get a warning”. Extra keywords for search: “struct is never constructed”.)

[bluenote10]

For the record, I'd asked a related question on SO, and the answer has pointed out that the issue apparently got worse over time, because in the meantime all cases in @shepmaster's example prevent the warning.