Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Casting or adding type ascription to panic!() triggers unreachable_code #67227

Open
leo60228 opened this issue Dec 11, 2019 · 7 comments
Open
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. F-never_type `#![feature(never_type)]` P-medium Medium priority requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@leo60228
Copy link
Contributor

fn test() -> impl Iterator<Item = i32> {
    panic!()
}

doesn't compile, as () (nor !) implement Iterator<Item = i32>.

#![feature(never_type_fallback)] // required for `panic!() as _`
fn test() -> impl Iterator<Item = i32> {
    panic!() as std::iter::Empty<_>
}

compiles, but gives this warning:

warning: unreachable expression
 --> src/lib.rs:3:5
  |
3 |     panic!() as std::iter::Empty<_>
  |     --------^^^^^^^^^^^^^^^^^^^^^^^
  |     |
  |     unreachable expression
  |     any code following this expression is unreachable
  |
  = note: `#[warn(unreachable_code)]` on by default
  = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
#![feature(type_ascription)]
fn test() -> impl Iterator<Item = i32> {
    panic!(): std::iter::Empty<_>
}

also compiles, but gives the same warning.

@jonas-schievink jonas-schievink added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. F-never_type `#![feature(never_type)]` labels Dec 11, 2019
@Centril
Copy link
Contributor

Centril commented Dec 12, 2019

Me and @jonas-schievink briefly discussed this on Discord.

It's not clear to me that this is a bug, but I could also see an argument for why coercions (through type ascription or via e.g. -> ReturnType { ... }) and possibly casts (as) would be treated specially as they "do nothing" (not entirely true, as casts and implicit coercions they may perform run-time work). As such, I'll relabel this as a language design question and if we agree that the behavior should change we can pass it back to the compiler team.


Particularly, we might consider coercions & casts different than e.g.

fn test() -> impl Iterator<Item = i32> {
    foo(
        panic!()
    )
}

fn foo(_: !) -> impl Iterator<Item = i32> { std::iter::empty() }

which results in:

warning: unreachable call
 --> src/lib.rs:2:5
  |
2 |     foo(
  |     ^^^ unreachable call
3 |         panic!()
  |         -------- any code following this expression is unreachable
  |
  = note: `#[warn(unreachable_code)]` on by default
  = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

@Centril Centril added T-lang Relevant to the language team, which will review and decide on the PR/issue. I-nominated and removed C-bug Category: This is a bug. labels Dec 12, 2019
@pnkfelix
Copy link
Member

T-compiler triage: Leaving unprioritized for now while we wait to see what T-lang decides about whether this is a bug or not.

@Centril
Copy link
Contributor

Centril commented Dec 13, 2019

We discussed this very briefly in Thursday's language team meeting. Sadly, we did not have quorum to make any decisions. Mainly, I informed those present of the possible argument for a distinction wrt. casts and coercions as compared to e.g. function calls.

@leo60228 If possible, it would be good to elaborate on why we should add a special case to stop linting here and whether such complexity would be justified.

@leo60228
Copy link
Contributor Author

I return unimplemented!() from all my WIP methods so I can cargo check while I'm writing them. I can't do this for a method returning impl Trait without triggering this lint.

@leo60228
Copy link
Contributor Author

While I could just return std::iter::empty or similar, I think unimplemented!() expressed my intent much more clearly.

@pnkfelix
Copy link
Member

While I am sympathetic to the annoyance of the lint here, this issue does not seem like a high priority issue for us to address.

triage: P-medium. Leaving nomination label so that we still try to discuss what to do next about it.

@pnkfelix pnkfelix added the P-medium Medium priority label Dec 19, 2019
@Centril
Copy link
Contributor

Centril commented Jan 9, 2020

The language team discussed this today. Attendance was low, but we agreed that it would be OK to avoid linting in this case as it “makes sense because these expressions are useful for ensuring type inference succeeds”. This would apply to both type ascription and casts.

@Centril Centril removed I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jan 9, 2020
@JohnTitor JohnTitor added C-bug Category: This is a bug. C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed C-bug Category: This is a bug. labels Feb 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. F-never_type `#![feature(never_type)]` P-medium Medium priority requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants