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

Missing "unreachable" warning for flow control in function argument #30289

Closed
diwic opened this issue Dec 9, 2015 · 8 comments
Closed

Missing "unreachable" warning for flow control in function argument #30289

diwic opened this issue Dec 9, 2015 · 8 comments
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut.

Comments

@diwic
Copy link
Contributor

diwic commented Dec 9, 2015

Consider this code:

fn foo(a: Option<i32>) -> i32 {
    a.unwrap_or({return 0 }) + 5
}   

In this case 0 is always returned (regardless of the value of a), which is probably not what the user intended. Because unwrap_or is never called, this should trigger an "unreachable code" or "dead code" warning, but it does not.

@durka
Copy link
Contributor

durka commented Dec 9, 2015

Hmm, I thought this was supposedly fixed in #30000.

@diwic
Copy link
Contributor Author

diwic commented Dec 9, 2015

@durka I just tried it with the "nightly" channel on play.rust-lang.org and it gave no warning, but maybe play.rust-lang-org is not updated since this was from 14 days ago.

@durka
Copy link
Contributor

durka commented Dec 9, 2015

Yeah, I meant "it was supposedly fixed, but it wasn't". It doesn't warn on current nightly either.

@diwic
Copy link
Contributor Author

diwic commented Dec 9, 2015

Ok, thanks for verifying!

@huonw huonw added the A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. label Jan 6, 2016
@huonw
Copy link
Member

huonw commented Jan 6, 2016

cc @Manishearth

@Manishearth
Copy link
Member

I tried the following

--- a/src/librustc_typeck/check/op.rs
+++ b/src/librustc_typeck/check/op.rs
@@ -18,6 +18,7 @@ use super::{
     method,
     FnCtxt,
 };
+use lint;
 use middle::def_id::DefId;
 use middle::ty::{Ty, HasTypeFlags, PreferMutLvalue};
 use syntax::ast;
@@ -71,6 +72,20 @@ pub fn check_binop<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
     check_expr(fcx, lhs_expr);
     let lhs_ty = fcx.resolve_type_vars_if_possible(fcx.expr_ty(lhs_expr));

+    if let Some(&lhs_ty) = fcx.inh.tables.borrow().node_types.get(&lhs_expr.id) {
+        if fcx.infcx().type_var_diverges(lhs_ty) {
+            fcx.ccx.tcx.sess.add_lint(lint::builtin::UNREACHABLE_CODE, rhs_expr.id,
+                                      rhs_expr.span, "unreachable expression".to_string());
+        }
+    }
+
+    if let Some(&rhs_ty) = fcx.inh.tables.borrow().node_types.get(&rhs_expr.id) {
+        if fcx.infcx().type_var_diverges(rhs_ty) {
+            fcx.ccx.tcx.sess.add_lint(lint::builtin::UNREACHABLE_CODE, expr.id,
+                                      expr.span, "unreachable operation".to_string());
+        }
+    }
+
     match BinOpCategory::from(op) {
         BinOpCategory::Shortcircuit => {
             // && and || are a simple case.

but it didn't work. Not sure why, haven't investigated.

@durka This wasn't supposed to be fixed in that issue, that was for method calls only.

@Ameobea
Copy link

Ameobea commented Oct 27, 2016

This seems to be working for a different case on the current nightly but the example used in the issue still doesn't report. Perhaps this can be used to find a solution:

@sinkuu
Copy link
Contributor

sinkuu commented Nov 16, 2016

This gets fixed in nightly.

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.
Projects
None yet
Development

No branches or pull requests

7 participants