uninhabited_references is very likely to be a false positive

[apoelstra]

Summary

The new uninhabited_reference lint complains about deferencing a reference to an infallible type, saying this is UB. But it is possible to write such code in safe Rust, which will be provably impossible to call (absent unsafe code which itself is almost certainly incorrect).

The lint purpose would be better served by linting the creation of a reference to an uninhabited type. The original issue has some discussion saying that it's maybe not UB, but it's almost certainly wrong. Versus the dereference, which is safe to write and has legitimate usecases.

At the very least, the existing lint shouldn't trigger on trait implementations.

cc #11851

Lint Name

No response

Reproducer

impl PushBytesErrorReport for core::convert::Infallible {
    #[inline]
    fn input_len(&self) -> usize { match *self {} }
}

triggers the lint. But this code is clearly implementing an error-related trait for Infallible, with the understanding that the trait method is impossible to call.

Version

No response

Additional Labels

No response

[Kixunil]

Also a different perspective: since unsafe code could create the reference then by the logic of this lint all dereferences should be warned against because they could be UB...

[kpreid]

The lint purpose would be better served by linting the creation of a reference to an uninhabited type.

T-opsem is debating the validity of references to uninhabited types at length.

(I agree that this lint should not be firing on match *my_ref {}; that's what I came here to report before I found it was already reported.)

[TethysSvensson]

I agree that this is a false positive. The lint should be disabled, or at least permit code in the form fn method(..., value: &Empty, ...) { match *my_ref {} } until the debate about operational semantics have been settled.

This pattern is used in a lot of places and as far as I know it is the idiomatic way of telling the compiler "this function cannot be called and all references to it should be optimized away". I do not know of any alternatives to this syntax in safe rust.

This pattern is especially useful when you have a lot of generic trait impls such as impl<T: SomeTrait> SomeTrait for Option<T> {}. If you want this impl to work with Option<Void>, then you need to be able to implement SomeTrait for Void. However you also want to tell the compiler that the function is bogus and should not be callable.

We have one such example in planus where we are currently considering disabling this lint.

[asquared31415]

As another data point, this is the implementation of every trait on ! in core, which is useful for being generic.

[xFrednet]

I'm reopening this issue, as the linked PR didn't actually resolve it but made the lint allow.by-default instead

[GrantGryczan]

This issue's title should be edited to add the s at the end of the lint name, so that this issue shows up under the "Related Issues" link of the lint in Clippy's docs.

[KisaragiEffective]

another case (minimized):

use core::convert::Infallible;

#[warn(clippy::uninhabited_references)]
pub fn ignore_infallible(x: &Result<u64, Infallible>) -> u64 {
  match x {
    Ok(v) => v + 1,
    Err(e) => {
      match *e {
      }
    }
  }
}

the lint does not like deref:

    Checking playground v0.0.1 (/playground)
warning: dereferencing a reference to an uninhabited type is undefined behavior
 --> src/lib.rs:8:13
  |
8 |       match *e {
  |             ^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninhabited_references
note: the lint level is defined here
 --> src/lib.rs:3:8
  |
3 | #[warn(clippy::uninhabited_references)]
  |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: `playground` (lib) generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.29s

[KisaragiEffective]

I will look into this once I get some time, I think we agreed that (at least) matching on uninhabited types itself is not UB (but creating reference to those is UB), so that entire operation must be problematic before it; implies that match should not trigger lint.

[KisaragiEffective]

@rustbot claim

[asquared31415]

EDIT: I checked some things in more depth, and it is invalid to create such a reference, sorry for the confusion.