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

Possibly bad example in documentation for 'unreachable!()' #18876

Closed
thecoshman opened this issue Nov 11, 2014 · 6 comments · Fixed by #19316
Closed

Possibly bad example in documentation for 'unreachable!()' #18876

thecoshman opened this issue Nov 11, 2014 · 6 comments · Fixed by #19316

Comments

@thecoshman
Copy link

At present, the documentation for unreachable!() has an example where you could not argue that the 'unreachable' code is indeed unreachable:

struct Item { weight: uint }

fn choose_weighted_item(v: &[Item]) -> Item {
    assert!(!v.is_empty());
    let mut so_far = 0u;
    for item in v.iter() {
        so_far += item.weight;
        if so_far > 100 {
            return *item;
        }
    }
    // The above loop always returns, so we must hint to the
    // type checker that it isn't possible to get down here
    unreachable!();
}

I would argue a better example would see a loop more like:

fn get_at_lest_100(){
    loop {
        sum += some_function_that_returns_unsigned_numbers();
        if sum > 100 { return sum; } 
    }
    unreachable!();
}

Perhaps I am miss understanding the purpose of this macro, in the example I provided here, is the compiler able to determine that the loop will keep going until eventually the return sum; is executed, and thus there is no need for the unreachable();. In which case, I think the documentation should make it clear that in such situations the macro is not required.

I'd argue it is as important to know where not to use this macro as it is when to use it.

@steveklabnik
Copy link
Member

This example certainly is more complicated than it has to be.

@Gankra
Copy link
Contributor

Gankra commented Nov 11, 2014

Your example literally warns that the unreachable is unreachable.

Rust understands that loop is forever.

@Gankra
Copy link
Contributor

Gankra commented Nov 11, 2014

unreachable is for asserting that while technically the control flow can reach this block of code, it would be a logic error for this to occur. The most common usage I know is for a pattern match which has some logical constraints that can't be expressed by the type system.

For example this simple function in collections::btree::node:

    /// Remove the key-value pair at the given index
    pub fn remove_as_leaf(&mut self, index: uint) -> (K, V) {
        match (self.keys.remove(index), self.vals.remove(index)) {
            (Some(k), Some(v)) => (k, v),
            _ => unreachable!(),
        }
    }

Here we are asserting that the program is in an inconsistent state if both removes yield anything other than Some.

Similarly you would use unreachable after a for loop if the program is incorrect if the loop terminates normally.

@huonw huonw added the A-docs label Nov 12, 2014
@thecoshman
Copy link
Author

@steveklabnik do you mean the example in the documentation or my proposed example is overly complicated?

@gankro ah, I see what it's purpose is now then. Perhaps the documentation should make it clear that there is no need to mark code that that literally cannot be reached, only code which shouldn't be reached. I think your example is much nicer as it shows a more real world example of where this macro should be used.

@steveklabnik
Copy link
Member

The exam;ple in the documentation.

@nodakai
Copy link
Contributor

nodakai commented Nov 13, 2014

On Reddit I learned exhaustiveness check can't understand match guards and we sometimes need to insert a seemingly unnecessary arm with wildcard pattern.

fn main() {
    match Some(1_i32) {
        Some(n) if n >= 0 => println!("Some(Non-negative)"),
        Some(n) if n <  0 => println!("Some(Negative)"),
        Some(_)           => unreachable!(), // compile error if commented out
        None              => println!("None")
    }
}

Iterator utilities sometimes need unreachable!():

fn divide_by_three(x: i32) -> i32 { // one of the poorest implementations of x/3
    for i in std::iter::count(0_i32, 1) {
        if i < 0 { panic!("i32 overflow"); }
        if x < 3*i { return i; }
    }
    unreachable!();
}

fn main() {
    println!("{}", divide_by_three(10));
}

steveklabnik added a commit to steveklabnik/rust that referenced this issue Nov 25, 2014
alexcrichton added a commit to alexcrichton/rust that referenced this issue Nov 27, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants