Lint uncallable function

[varkor]

Functions with parameters whose types are uninhabited now fall under the unreachable_code lint, unless they're part of a trait implementation, as these are currently necessary.

r? @nikomatsakis

[Mark-Simulacrum]

Hm, I think I recall seeing that syn is somehow using functions or types like this as arguments that are never called but used for trait based dispatch.. cc @dtolnay

(Not sure whether there's actually any concern to be had here but want to make sure)

[petrochenkov]

Is this something that needs to be linted at all?
What's the chance of using an uninhabited type accidentally in a public function's signature and then not attempting to use that function?

[varkor]

My motivation for adding this case to the lint is bringing the unreachable code/divergence checking in typeck closer to the unreachable code checking for MIR. I imagine it's not that common, but seems to naturally extend the existing lint and at least provide a warning that an entire function may be eliminated during codegen.

[dtolnay]

Thanks for the ping @Mark-Simulacrum. Yes we use this.
I would just add #[allow(unreachable_code)] if there were a lint, so not a big deal.

[bors]

☔ The latest upstream changes (presumably #53508) made this pull request unmergeable. Please resolve the merge conflicts.

[XAMPPRocky]

Triage; @nikomatsakis Hello, are you able to review this PR?

[nikomatsakis]

I've not been reviewing this because it is marked as blocked on #54125, which has not yet landed.

[bors]

☔ The latest upstream changes (presumably #54667) made this pull request unmergeable. Please resolve the merge conflicts.

[varkor]

This now no longer depends on #54125.

[TimNN]

Ping from triage @nikomatsakis: It looks like this PR is ready for your review.

[bors]

☔ The latest upstream changes (presumably #56340) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

📌 Commit adac7a1 has been approved by nikomatsakis

[bors]

⌛ Testing commit adac7a1 with merge fe7b85910c7e42bdcfd75d53a15ab74acc9ec0dd...

[bors]

💔 Test failed - status-travis

[rust-highfive]

The job x86_64-gnu-aux of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[01:36:32] [RUSTC-TIMING] rand test:false 4.597
[01:36:32]    Compiling tempfile v3.0.5
[01:36:33] [RUSTC-TIMING] tempfile test:false 1.440
[01:36:33]    Compiling cargo v0.34.0 (/checkout/src/tools/cargo)
[01:36:39] warning: functions with parameters of uninhabited types are uncallable
[01:36:39]    |
[01:36:39]    |
[01:36:39] 32 | #[derive(Deserialize, Serialize)]
[01:36:39]    |          ^^^^^^^^^^^ this parameter has an uninhabited type
[01:36:39]    = note: #[warn(unreachable_code)] on by default
[01:36:39] 
[01:39:40] [RUSTC-TIMING] cargo test:false 187.146
[01:40:02] [RUSTC-TIMING] cargo test:false 21.848
---
[01:40:08] [RUSTC-TIMING] num_traits test:false 3.325
[01:40:08]    Compiling proptest v0.8.7
[01:40:31] [RUSTC-TIMING] proptest test:false 22.595
[01:40:31]    Compiling cargo v0.34.0 (/checkout/src/tools/cargo)
[01:40:37] error: functions with parameters of uninhabited types are uncallable
[01:40:37]    |
[01:40:37]    |
[01:40:37] 32 | #[derive(Deserialize, Serialize)]
[01:40:37]    |          ^^^^^^^^^^^ this parameter has an uninhabited type
[01:40:37] note: lint level defined here
[01:40:37]   --> src/tools/cargo/src/cargo/lib.rs:1:24
[01:40:37]    |
[01:40:37] 1  | #![cfg_attr(test, deny(warnings))]
[01:40:37] 1  | #![cfg_attr(test, deny(warnings))]
[01:40:37]    |                        ^^^^^^^^
[01:40:37]    = note: #[deny(unreachable_code)] implied by #[deny(warnings)]
[01:40:40] error: aborting due to previous error
[01:40:40] 
[01:40:40] [RUSTC-TIMING] cargo test:true 9.146
[01:40:40] error: Could not compile `cargo`.
---
[01:41:56] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--manifest-path" "/checkout/src/tools/cargo/Cargo.toml" "--features" "rustc-workspace-hack/all-static"
[01:41:56] expected success, got: exit code: 101
[01:41:56] 
[01:41:56] 
[01:41:56] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/test/pretty src/test/run-pass/pretty src/test/run-fail/pretty src/test/run-pass-valgrind/pretty src/test/run-pass-fulldeps/pretty src/test/run-fail-fulldeps/pretty src/tools/cargo src/tools/cargotest
[01:41:56] Build completed unsuccessfully in 0:27:20
[01:41:56] Makefile:50: recipe for target 'check-aux' failed
[01:41:56] make: *** [check-aux] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0582ca71
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Thu Jan  3 17:17:32 UTC 2019
---
travis_time:end:1bce0610:start=1546535854317070366,finish=1546535854324086901,duration=7016535
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:00130110
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0d229152
travis_time:start:0d229152
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:02a1e288
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

[nikomatsakis]

[01:36:39] warning: functions with parameters of uninhabited types are uncallable
[01:36:39]    |
[01:36:39]    |
[01:36:39] 32 | #[derive(Deserialize, Serialize)]
[01:36:39]    |          ^^^^^^^^^^^ this parameter has an uninhabited type
[01:36:39]    = note: #[warn(unreachable_code)] on by default

[nikomatsakis]

Not sure what code is triggering that, seems worth finding out though.

[varkor]

The code that's triggering this is:
https://github.com/rust-lang/cargo/blob/0d1f1bbeabd5b43a7f3ecfa16540af8e76d5efb4/src/cargo/ops/cargo_install.rs#L32-L34

#[derive(Deserialize, Serialize)]
#[serde(deny_unknown_fields)]
struct Empty {}

Deserialize is generating a function taking an Empty, which is uncallable and thus triggering the warning. Probably the simplest thing to do is to ignore this case and not trigger the warning in functions expanded from derive.

[Mark-Simulacrum]

Wait, how is that function uncallable? That Empty struct can be created, right? An enum would be uninhabited but an empty struct is zero-sized, not uninhabited.

[varkor]

You're right — I misread it as enum. I imagine it's still something to do with a derived function that somehow considers the fields of the struct, but I'll need to look into it.

[stokhos]

Ping from triage @varkor : Have you been able to make any progress on this PR?

[varkor]

Not yet; it's been a busy couple of weeks. I shall try to do so soon.

[Centril]

@bors r-

Not sure why this is r+ed...

[Centril]

As for the PR itself; I am not at all sure this is "unobjectionable". In particular, functions such as fn absurd<T>(x: !) -> T { x } can be useful when passed to higher order functions. I've therefore marked this as T-Lang, etc.

[varkor]

@Centril: can you give a real example?

[Centril]

@varkor Certainly; For example, given foo: Result<?T, !>, foo.unwrap_or_else(absurd). Moreover, this function absurd :: Void -> a is part of the Haskell package void which has a non-trivial amount of usage. Also, in discussions about stabilizing !, ?E: From<!> was discussed as useful, and that is equivalent to absurd.

[varkor]

@Centril: I think it's always sufficient in such an instance to use an identity function, without explicitly mentioning !.

[Centril]

@varkor If that identity function is a closure, then yes, e.g.

#![feature(never_type)]

fn main() {
    fn make() -> Result<u8, !> { Ok(1) }
    fn absurd<T>(x: !) -> T { x }

    let _: u8 = make().unwrap_or_else(absurd); // Works fine.
    let _: u8 = make().unwrap_or_else(|x| x); // Also works fine because of coercions.
    let _: u8 = make().unwrap_or_else(core::convert::identity); // Does not.
}

I think writing absurd like that explicitly is more clear rather than intuiting the behavior from the closure so to me the lint would have false positives, and we typically stay away from those in rustc.

[varkor]

I can see that uncallable functions are plausibly useful from a documentation perspective. I think I'd be satisfied making functions taking never types as parameters immediately diverging, so that the following would work.

fn bar(_: !) -> u32 {
    // Implicitly diverging, because an argument is `!`.
}

Then we can start warning against any other code in such an uncallable function. (Uncallable functions that use some type other than ! would still have to explicitly match on the argument to be diverging.)

[varkor]

I'm going to close this PR, as it's extremely low priority for the lang team to look at, and keeping it open indefinitely is not useful.