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

Unused variable warning for used variable. #85071

Closed
koehlma opened this issue May 8, 2021 · 7 comments · Fixed by #85556
Closed

Unused variable warning for used variable. #85071

koehlma opened this issue May 8, 2021 · 7 comments · Fixed by #85556
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@koehlma
Copy link
Contributor

koehlma commented May 8, 2021

When trying to compile the following code I get an unused variable warning despite the variable is being used:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=66167b96f0e14d53616abbd0e6b3d12c

The current output is:

warning: unused variable: `typ`
  --> src/lib.rs:62:17
   |
62 |             let typ = T::infer_type(self);
   |                 ^^^ help: if this is intentional, prefix it with an underscore: `_typ`
   |
   = note: `#[warn(unused_variables)]` on by default

Ideally the output should not produce such a warning.

cargo fix produces the following output:

warning: failed to automatically apply fixes suggested by rustc to crate `rust_playground`

after fixes were automatically applied the compiler reported errors within these files:

  * src\lib.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

The following errors were reported:
error[E0425]: cannot find value `typ` in this scope
  --> src\lib.rs:64:30
   |
64 |             info.typ.replace(typ);
   |                              ^^^ help: a local variable with a similar name exists: `_typ`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0425`.
Original diagnostics will follow.

warning: unused variable: `typ`
  --> src\lib.rs:62:17
   |
62 |             let typ = T::infer_type(self);
   |                 ^^^ help: if this is intentional, prefix it with an underscore: `_typ`
   |
   = note: `#[warn(unused_variables)]` on by default

warning: 1 warning emitted

warning: unused variable: `typ`
  --> src\lib.rs:62:17
   |
62 |             let typ = T::infer_type(self);
   |                 ^^^ help: if this is intentional, prefix it with an underscore: `_typ`
   |
   = note: `#[warn(unused_variables)]` on by default

warning: 1 warning emitted

    Finished dev [unoptimized + debuginfo] target(s) in 1.04s
@koehlma koehlma added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 8, 2021
@koehlma
Copy link
Contributor Author

koehlma commented May 8, 2021

The reason seems to be that enum Type does not have any variants.

When I add a variant to the enum the warning disappears.

@ehuss
Copy link
Contributor

ehuss commented May 8, 2021

Yea, since an empty enum cannot be created, I presume the compiler assumes all of the code after the call to infer_type cannot be run, so it does not exist. So, in a way, typ cannot be used because it cannot be constructed. I kinda wonder why it didn't give an unreachable statement warning instead. Anyways, yea, the warning could be better.

A little smaller repro:

#[derive(Debug)]
enum Foo {}
fn f() -> Foo {todo!()}
let x = f();
println!("{:?}", x);

@FabianWolff
Copy link
Contributor

Yes, this behavior seems to come from this part of the liveness analysis:

hir::ExprKind::Call(ref f, ref args) => {
let m = self.ir.tcx.parent_module(expr.hir_id).to_def_id();
let succ = if self.ir.tcx.is_ty_uninhabited_from(
m,
self.typeck_results.expr_ty(expr),
self.param_env,
) {
self.exit_ln
} else {
succ
};
let succ = self.propagate_through_exprs(args, succ);
self.propagate_through_expr(&f, succ)
}
hir::ExprKind::MethodCall(.., ref args, _) => {
let m = self.ir.tcx.parent_module(expr.hir_id).to_def_id();
let succ = if self.ir.tcx.is_ty_uninhabited_from(
m,
self.typeck_results.expr_ty(expr),
self.param_env,
) {
self.exit_ln
} else {
succ
};
self.propagate_through_exprs(args, succ)
}

i.e. the compiler realizes that a call to a method returning an uninhabited type can never return (it has to either panic or be non-terminating), and so it discards everything after the call for the purposes of liveness analysis. I agree that the warnings about unused variables, while technically correct, are somewhat confusing, and that there should probably be a warning about unreachable statements after the call.

@koehlma
Copy link
Contributor Author

koehlma commented May 8, 2021

Makes sense. Yes, an unreachable statement warning for the remainder of the code would be very helpful. 👌

@Rustin170506
Copy link
Member

@rustbot claim

Let me try to add a warning.

@FabianWolff
Copy link
Contributor

Let me try to add a warning.

Good luck, though I tried to look into this yesterday and concluded that there is no easy way to do this. Checking for unreachable expressions is integrated into the type checker, as divergence can influence type inference, e.g. here:

// Subtle: if there is no explicit tail expression,
// that is typically equivalent to a tail expression
// of `()` -- except if the block diverges. In that
// case, there is no value supplied from the tail
// expression (assuming there are no other breaks,
// this implies that the type of the block will be
// `!`).

Unfortunately, this means that full type information is not yet available at the point where the warnings for unreachable statements are generated. The easy solution would have been to add a call to tcx.is_ty_uninhabited_from() here:
// Any expression that produces a value of type `!` must have diverged
if ty.is_never() {
self.diverges.set(self.diverges.get() | Diverges::always(expr.span));
}

Predictably, though, this doesn't work, because type information hasn't been stored in tcx yet (see here). It doesn't even crash, but leads to a broken (non-bootstrappable) compiler.

One possibility would be to move warnings about unreachable code into a separate pass, but then there would be code duplication between the type checker's divergence checking and this separate pass; maybe that is worth it if there is some other benefit, but functions returning uninhabited types seems like too much of an edge case to be worth it.

I don't mean to discourage you, though — maybe you can find a better solution!

@Rustin170506
Copy link
Member

I can't fix it. So unassign myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants