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

Unclear Error Message in Philosopher Example #25488

Closed
StefanoD opened this issue May 16, 2015 · 5 comments · Fixed by #30595
Closed

Unclear Error Message in Philosopher Example #25488

StefanoD opened this issue May 16, 2015 · 5 comments · Fixed by #30595
Labels
A-diagnostics Area: Messages for errors, warnings, and lints

Comments

@StefanoD
Copy link

This code...

use std::thread;

struct Philosopher {
    name: String,
}

impl Philosopher {
    fn new(name: &str) -> Philosopher {
        Philosopher {
            name: name.to_string(),
        }
    }

    fn eat(&self) {
        println!("{} is eating.", self.name);

        thread::sleep_ms(1000);

        println!("{} is done eating.", self.name);
    }
}

fn main() {
    let philosophers = vec![
        Philosopher::new("Baruch Spinoza"),
        Philosopher::new("Gilles Deleuze"),
        Philosopher::new("Karl Marx"),
        Philosopher::new("Friedrich Nietzsche"),
        Philosopher::new("Michel Foucault"),
    ];

    let handles: Vec<_> = philosophers.into_iter().map(|p| {
        thread::spawn(move || {
            p.eat();
        });
    }).collect();

    for h in handles {
        h.join().unwrap();
    }
}

... leads to:

src/main.rs:39:11: 39:17 error: type `()` does not implement any method in scope named `join`
src/main.rs:39         h.join().unwrap();
                         ^~~~~~
error: aborting due to previous error
Could not compile `Philosopher`.

Actually the error lies in line 35 in the semicolon }); at the end:

thread::spawn(move || {
            p.eat();
        });
@pnkfelix
Copy link
Member

That's an interesting failure mode; i.e. you were forced to say the Vec type explicitly, but the language lets you leave out the element type via Vec<_>. And then it inferred that the element type is () since that is the result of the map callback, due to the semicolon after thread::spawn that was noted in the description...

error reporting for type inference errors is a notoriously difficult area, but we probably could do more here. Even just special casing (), since the presence/absence of a semi-colon is an easy thing to overlook, might be useful.

(by "special-casing ()", I mean something hacky like tracking when we infer a unification-variable to be (), and then if that ends up being involved with a type error down the road, then report the chain of inference decisions that led to the () being introduced.)

@steveklabnik steveklabnik added the A-diagnostics Area: Messages for errors, warnings, and lints label May 18, 2015
@ElderThorin
Copy link

Removing the ; at the end of the

thread::spawn(move || {
        p.eat();
    });

(the current state of the documentation does not have a semicolon in that location)

results in the following slightly different compile error, and thus the current version of the dining philosophers program does not compile. As I'm just trying to follow the examples for the first time, I have no clue why unwrap would or would not be available.

main.rs:39:9: 39:24 error: attempted access of field `unwrap` on type `core::result::Result<(), Box<core::any::Any + Send>>`, but no field with that name was found
main.rs:39         h.join().unwrap;
                   ^~~~~~~~~~~~~~~
error: aborting due to previous error

It's possible this should be a separate issue, I'm not certain.

Note: this fails in this way with the Windows 64bit 1.0.0-beta build (built 2015-04-02)

@pnkfelix
Copy link
Member

@tlinderh it looks to me like you removed the () on the end of .unwrap(); as well, turning an attempted method call to an .unwrap() method into an a field access on the (non-existent) .unwrap field.

Update: (If you are using something other than the source code in this ticket description as the basis for your comment, then you should probably include a link in your comment (and, as you mention, perhaps file a separate issue)...)

@ElderThorin
Copy link

I'm not certain how I ended up pasting in a version without () at the end of unwrap(). That's not even the correct error message. It should read '... does not implement any method in scope named 'unwrap'. This was after copy-pasting the code directly out of the rust dining philosophers example (the last version of it listed.)

http://doc.rust-lang.org/stable/book/dining-philosophers.html

It works fine in the rust web workshop thing, but it doesn't work with the windows 6bit .0.0-beta install on my machine.

Opened new issue #25649 for this issue.

@benfleis
Copy link
Contributor

I just ran into this, and what isn't clear from either the book, or the error, is why a semi-colon isn't expected. Perhaps this is obvious to Rustaceans, but as a complete newcomer, it's counter intuitive to not have a ; in that location, and the produced error left me scratching my head until I gave up and diff'd my code versus the given code from the book.

Thus, +1 on an improved error message, if it's reasonable.
And +1 on a bit of explanation in the book -- if the semi-colon changes the return type of a lambda, it seems worthy of more :)

benfleis added a commit to benfleis/rust that referenced this issue Jun 2, 2015
Manishearth added a commit to Manishearth/rust that referenced this issue Jun 9, 2015
steveklabnik added a commit to steveklabnik/rust that referenced this issue Dec 28, 2015
Some history:

While getting Rust to 1.0, it was a struggle to keep the book in a
working state. I had always wanted a certain kind of TOC, but couldn't
quite get it there.

At the 11th hour, I wrote up "Rust inside other langauges" and "Dining
Philosophers" in an attempt to get the book in the direction I wanted to
go. They were fine, but not my best work. I wanted to further expand
this section, but it's just never going to end up happening. We're doing
the second draft of the book now, and these sections are basically gone
already.

Here's the issues with these two sections, and removing them just fixes
it all:

// Philosophers

There was always controversy over which ones were chosen, and why. This
is kind of a perpetual bikeshed, but it comes up every once in a while.

The implementation was originally supposed to show off channels, but
never did, due to time constraints. Months later, I still haven't
re-written it to use them.

People get different results and assume that means they're wrong, rather
than the non-determinism inherent in concurrency. Platform differences
aggrivate this, as does the exact amount of sleeping and printing.

// Rust Inside Other Languages

This section is wonderful, and shows off a strength of Rust. However,
it's not clear what qualifies a language to be in this section. And I'm
not sure how tracking a ton of other languages is gonna work, into the
future; we can't test _anything_ in this section, so it's prone to
bitrot.

By removing this section, and making the Guessing Game an initial
tutorial, we will move this version of the book closer to the future
version, and just eliminate all of these questions.

In addition, this also solves the 'split-brained'-ness of having two
paths, which has endlessly confused people in the past.

I'm sad to see these sections go, but I think it's for the best.

Fixes rust-lang#30471
Fixes rust-lang#30163
Fixes rust-lang#30162
Fixes rust-lang#25488
Fixes rust-lang#30345
Fixes rust-lang#29590
Fixes rust-lang#28713
Fixes rust-lang#28915

And probably others. This lengthy list alone is enough to show that
these should have been removed.

RIP.
Manishearth added a commit to Manishearth/rust that referenced this issue Dec 29, 2015
…ankro

Some history:

While getting Rust to 1.0, it was a struggle to keep the book in a
working state. I had always wanted a certain kind of TOC, but couldn't
quite get it there.

At the 11th hour, I wrote up "Rust inside other langauges" and "Dining
Philosophers" in an attempt to get the book in the direction I wanted to
go. They were fine, but not my best work. I wanted to further expand
this section, but it's just never going to end up happening. We're doing
the second draft of the book now, and these sections are basically gone
already.

Here's the issues with these two sections, and removing them just fixes
it all:

// Philosophers

There was always controversy over which ones were chosen, and why. This
is kind of a perpetual bikeshed, but it comes up every once in a while.

The implementation was originally supposed to show off channels, but
never did, due to time constraints. Months later, I still haven't
re-written it to use them.

People get different results and assume that means they're wrong, rather
than the non-determinism inherent in concurrency. Platform differences
aggrivate this, as does the exact amount of sleeping and printing.

// Rust Inside Other Languages

This section is wonderful, and shows off a strength of Rust. However,
it's not clear what qualifies a language to be in this section. And I'm
not sure how tracking a ton of other languages is gonna work, into the
future; we can't test _anything_ in this section, so it's prone to
bitrot.

By removing this section, and making the Guessing Game an initial
tutorial, we will move this version of the book closer to the future
version, and just eliminate all of these questions.

In addition, this also solves the 'split-brained'-ness of having two
paths, which has endlessly confused people in the past.

I'm sad to see these sections go, but I think it's for the best.

Fixes rust-lang#30471
Fixes rust-lang#30163
Fixes rust-lang#30162
Fixes rust-lang#25488
Fixes rust-lang#30345
Fixes rust-lang#29590
Fixes rust-lang#28713
Fixes rust-lang#28915

And probably others. This lengthy list alone is enough to show that
these should have been removed.

RIP.
steveklabnik added a commit to steveklabnik/rust that referenced this issue Dec 29, 2015
Some history:

While getting Rust to 1.0, it was a struggle to keep the book in a
working state. I had always wanted a certain kind of TOC, but couldn't
quite get it there.

At the 11th hour, I wrote up "Rust inside other langauges" and "Dining
Philosophers" in an attempt to get the book in the direction I wanted to
go. They were fine, but not my best work. I wanted to further expand
this section, but it's just never going to end up happening. We're doing
the second draft of the book now, and these sections are basically gone
already.

Here's the issues with these two sections, and removing them just fixes
it all:

// Philosophers

There was always controversy over which ones were chosen, and why. This
is kind of a perpetual bikeshed, but it comes up every once in a while.

The implementation was originally supposed to show off channels, but
never did, due to time constraints. Months later, I still haven't
re-written it to use them.

People get different results and assume that means they're wrong, rather
than the non-determinism inherent in concurrency. Platform differences
aggrivate this, as does the exact amount of sleeping and printing.

// Rust Inside Other Languages

This section is wonderful, and shows off a strength of Rust. However,
it's not clear what qualifies a language to be in this section. And I'm
not sure how tracking a ton of other languages is gonna work, into the
future; we can't test _anything_ in this section, so it's prone to
bitrot.

By removing this section, and making the Guessing Game an initial
tutorial, we will move this version of the book closer to the future
version, and just eliminate all of these questions.

In addition, this also solves the 'split-brained'-ness of having two
paths, which has endlessly confused people in the past.

I'm sad to see these sections go, but I think it's for the best.

Fixes rust-lang#30471
Fixes rust-lang#30163
Fixes rust-lang#30162
Fixes rust-lang#25488
Fixes rust-lang#30345
Fixes rust-lang#28713
Fixes rust-lang#28915

And probably others. This lengthy list alone is enough to show that
these should have been removed.

RIP.
bors added a commit that referenced this issue Jan 5, 2016
Some history:

While getting Rust to 1.0, it was a struggle to keep the book in a
working state. I had always wanted a certain kind of TOC, but couldn't
quite get it there.

At the 11th hour, I wrote up "Rust inside other langauges" and "Dining
Philosophers" in an attempt to get the book in the direction I wanted to
go. They were fine, but not my best work. I wanted to further expand
this section, but it's just never going to end up happening. We're doing
the second draft of the book now, and these sections are basically gone
already.

Here's the issues with these two sections, and removing them just fixes
it all:

// Philosophers

There was always controversy over which ones were chosen, and why. This
is kind of a perpetual bikeshed, but it comes up every once in a while.

The implementation was originally supposed to show off channels, but
never did, due to time constraints. Months later, I still haven't
re-written it to use them.

People get different results and assume that means they're wrong, rather
than the non-determinism inherent in concurrency. Platform differences
aggrivate this, as does the exact amount of sleeping and printing.

// Rust Inside Other Languages

This section is wonderful, and shows off a strength of Rust. However,
it's not clear what qualifies a language to be in this section. And I'm
not sure how tracking a ton of other languages is gonna work, into the
future; we can't test _anything_ in this section, so it's prone to
bitrot.

By removing this section, and making the Guessing Game an initial
tutorial, we will move this version of the book closer to the future
version, and just eliminate all of these questions.

In addition, this also solves the 'split-brained'-ness of having two
paths, which has endlessly confused people in the past.

I'm sad to see these sections go, but I think it's for the best.

Fixes #30471
Fixes #30163
Fixes #30162
Fixes #25488
Fixes #30345
Fixes #29590
Fixes #28713
Fixes #28915

And probably others. This lengthy list alone is enough to show that
these should have been removed.

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

Successfully merging a pull request may close this issue.

5 participants