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

Ran exercises/option/option2.rs with errors #424

Closed
alexxroche opened this issue Jun 5, 2020 · 6 comments
Closed

Ran exercises/option/option2.rs with errors #424

alexxroche opened this issue Jun 5, 2020 · 6 comments

Comments

@alexxroche
Copy link
Contributor

alexxroche commented Jun 5, 2020

I had rustlings watch running in one screen and was editing options2.rs in the other. After saving my attempt my operating system ground to a sticky halt, (it felt like a forkbomb.)

When I managed to restart rustlings watch the output was simply:

! Ran exercises/option/option2.rs with errors
current value: 9
current value: 8
current value: 7
current value: 6
current value: 5
current value: 4
current value: 3
current value: 2
current value: 1

So rustlings was telling me that there were errors, but rustc wasn't being its usual helpful self, and was silent about what those errors might be.

[I finished the exercise but went back through my branch to dig this out and report it.]

This was during the second half of the options2.rs exercise, so I trimmed out everything else and it was still happening, (example broken code folded away in the [SPOILER]).

[SPOILER] example BAD code
//exercises/option/option2.rs
fn main() {
    let mut optional_values_vec: Vec<Option<i8>> = Vec::new();
    for x in 1..10 {
        optional_values_vec.push(Some(x));
    }
    while let Some(value) = optional_values_vec.pop().unwrap() {
        println!("current value: {:?}", value);
    }
}

As this is meant to be teaching, I would have liked a little more guidance at that point to indicate what my mistake was. (Though hint was still functioning and was all that I needed to finish the exercise, I still don't know what was causing the "! Ran ... with errors".)

My rust/rustlings/OS version info
$ rustc --version
rustc 1.43.1 (8d69840ab 2020-05-04)
$ rustlings --version
rustlings 3.0.0
$ uname -a
Linux laptop 4.12.0-1-amd64 #1 SMP Debian 4.12.6-1 (2017-08-12) x86_64 GNU/Linux
@AbdouSeck
Copy link
Contributor

Hi @alexxroche, thanks for bringing this up. It is indeed a bug (more like an omission) at line 56 in src/verify.rs.

Basically, when the program is running interactively (i.e. using watch), only the standard output is printed when there is an error.

Adding println!("{}", output.stderr); should show you the usual panic message from rustc.

Please feel free to submit a pull request. Feel free to reach out if you have any questions.

Thanks,

Abdou

@alexxroche
Copy link
Contributor Author

That is an improvement. (I tried a few variations of your patch.)

Adding your suggestion (inserting an additional println!("{}", output.stderr); before the println!("{}", output.stdout); on line 56 in src/verify.rs
resulted in the (I believe to be very helpful) "Type 'hint' to get help" message. (I also tried inserting the stderr after the stdout (inserting on line 57) and had the same result. (Replacing the stdout with stderr added the hint message, but suppressed the output of option2.rs - which isn't ideal.)

So I agree that you have found an improvement, (for my case; I don't know enough about this program to know if printing to both stdout and stderr would harm any of the other exercises or would create a problematic edge case somewhere else.)

I still have the issue that if I compile options2.rs by hand I get a warning and running the resulting binary throws a, (useful and informative) panic:

cmdline compile by hand
$ rustc exercises/option/option2.rs 
warning: irrefutable if-let pattern
  --> exercises/option/option2.rs:9:5
   |
9  | /     if let value = optional_value.unwrap() {
10 | |         println!("the value of optional value is: {}", value);
11 | |     } else {
12 | |         println!("The optional value doesn't contain anything!");
13 | |     }
   | |_____^
   |
   = note: `#[warn(irrefutable_let_patterns)]` on by default
... and run example showing panic
$ ./option2 1>/dev/null
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', exercises/option/option2.rs:36:29
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I can understand suppressing the irrefutable_let_patterns but the panic message seems relevant and helpful, and worth showing to the user.

alexxroche added a commit to alexxroche/rustlings that referenced this issue Jun 6, 2020
Suggestion from AbdouSeck rust-lang#424 (comment)
for when the student's code has errors.
@AbdouSeck
Copy link
Contributor

Hi @alexxroche the missing warnings are also another omission because the assumption by rustlings is that if the source code compiles, there is no need to inspect the standard error of the child process that compiled the code. If you look at line 128 in src/exercise.rs, you'll notice that just because the child process came back with a success, no inspection of its standard error is done.

If you want the warnings to show, you would have to add a few lines below that (inside the if statement) to display the standard error if it's not empty. So that if statement can be rewritten as something like the following:

        if cmd.status.success() {
            // Because compiler warnings go to the standard error, we have to inspect the
            // standard error of the child process responsible for compiling the code.
            if !cmd.stderr.is_empty() {
                println!("{}", String::from_utf8_lossy(&cmd.stderr).to_string());
            }
            Ok(CompiledExercise {
                exercise: &self,
                _handle: FileHandle,
            })
        } else {
            clean();
            Err(ExerciseOutput {
                stdout: String::from_utf8_lossy(&cmd.stdout).to_string(),
                stderr: String::from_utf8_lossy(&cmd.stderr).to_string(),
            })
        }

Please give that a try and let me know what you think.

Thank you,

Abdou

alexxroche added a commit to alexxroche/rustlings that referenced this issue Jun 6, 2020
@alexxroche
Copy link
Contributor Author

My attempt to apply your patch to src/exercise.rs (near line 128) compiled but didn't change the output of rustlings watch or rustlings verify

I tried fs::write!() as well as println!() and neither managed to output to my terminal.

@AbdouSeck
Copy link
Contributor

AbdouSeck commented Jun 6, 2020

Hi @alexxroche I added your fork as remote and used it. I was able to see the changes I mentioned earlier upon compiling and calling cargo run -- watch. The following is the output I see:

⠙ Compiling exercises/option/option2.rs...
warning: irrefutable if-let pattern
  --> exercises/option/option2.rs:9:5
   |
9  | /     if let value = optional_value.unwrap() {
10 | |         println!("the value of optional value is: {}", value);
11 | |     } else {
12 | |         println!("The optional value doesn't contain anything!");
13 | |     }
   | |_____^
   |
   = note: `#[warn(irrefutable_let_patterns)]` on by default

⚠️  Ran exercises/option/option2.rs with errors
the value of optional value is: rustlings
current value is 9
current value is 8
current value is 7
current value is 6
current value is 5
current value is 4
current value is 3
current value is 2
current value is 1

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', exercises/option/option2.rs:22:29
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I am really not sure what could be the issue. But the following is my setup:

  • OS:
Darwin Kernel Version 19.5.0: Tue May 26 20:41:44 PDT 2020; root:xnu-6153.121.2~2/RELEASE_X86_64 x86_64
  • Cargo:
cargo 1.43.0 (3532cf738 2020-03-17)
  • rustc:
rustc 1.43.0 (4fb7144ed 2020-04-20)
  • git branch -v:
* (HEAD detached at alexxroche/master) 1bc567b Merge pull request #1 from alexxroche/alexxroche-patch-1
  • git remote -v:
alexxroche      [email protected]:alexxroche/rustlings.git (fetch)
alexxroche      [email protected]:alexxroche/rustlings.git (push)

Not quite sure what else to suggest other than that you make sure that you have compiled before running?

Thank you,

Abdou

@alexxroche
Copy link
Contributor Author

Thank you @AbdouSeck for looking at this issue. I think this could be considered a minor edge case that may interest someone that wants to specialise in UX. I'm going to move on with more of the exercises.

shadows-withal pushed a commit that referenced this issue Jul 8, 2020
Suggestion from AbdouSeck #424 (comment)
for when the student's code has errors.
shadows-withal pushed a commit that referenced this issue Jul 8, 2020
Suggestion from AbdouSeck #424 (comment)
for when the student's code has errors.
dlemel8 pushed a commit to dlemel8/rustlings that referenced this issue Aug 2, 2020
Suggestion from AbdouSeck rust-lang#424 (comment)
for when the student's code has errors.
ppp3 pushed a commit to ppp3/rustlings that referenced this issue May 23, 2022
Suggestion from AbdouSeck rust-lang#424 (comment)
for when the student's code has errors.
ppp3 pushed a commit to ppp3/rustlings that referenced this issue May 23, 2022
Suggestion from AbdouSeck rust-lang#424 (comment)
for when the student's code has errors.
FancyGeekGuru added a commit to FancyGeekGuru/rust-lings that referenced this issue May 24, 2022
Suggestion from AbdouSeck rust-lang/rustlings#424 (comment)
for when the student's code has errors.
FancyGeekGuru added a commit to FancyGeekGuru/rust-lings that referenced this issue May 24, 2022
Suggestion from AbdouSeck rust-lang/rustlings#424 (comment)
for when the student's code has errors.
dmoore04 pushed a commit to dmoore04/rustlings that referenced this issue Sep 11, 2022
Suggestion from AbdouSeck rust-lang#424 (comment)
for when the student's code has errors.
dmoore04 pushed a commit to dmoore04/rustlings that referenced this issue Sep 11, 2022
Suggestion from AbdouSeck rust-lang#424 (comment)
for when the student's code has errors.
kainlite pushed a commit to kainlite/rustlings that referenced this issue Dec 9, 2024
Suggestion from AbdouSeck rust-lang/rustlings#424 (comment)
for when the student's code has errors.
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

No branches or pull requests

3 participants