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

Update the behaviour of the compiler for code that will panic at runtime #2522

Closed
wants to merge 10 commits into from
Closed

Conversation

DoubleHyphen
Copy link

Currently, the compiler output clearly displays a compilation error, and refuses to compile code that would clearly panic at runtime. Despite that, the text around the example still speaks as if the code will compile without error and wait until runtime to panic. Therefore, I have updated the text, and included a more convoluted example that the compiler cannot currently catch.

@DoubleHyphen
Copy link
Author

@oli-obk @steveklabnik I was advised to ping you fine gentlemen. This is a small problem, with a solution that was a bit more involved than I had originally expected. I hope it won't take too much of your time.

@DoubleHyphen
Copy link
Author

Aaaaaaaand it's a duplicate of #2454. My bad.

Still, it's not an exact duplicate, as that PR does not contain any code displaying panic-at-runtime behaviour.

@carols10cents
Copy link
Member

Hi, thank you for working on this! I agree this section needs to be updated, but there's way too much complexity in the example you've provided to be included so early in the book as it is. I'm going to need some time to think about how we should handle this, because as you said, the point of this example is to show that some safety checks happen at runtime.

@DoubleHyphen
Copy link
Author

DoubleHyphen commented Dec 23, 2020

Hi, thank you for working on this! I agree this section needs to be updated, but there's way too much complexity in the example you've provided to be included so early in the book as it is. I'm going to need some time to think about how we should handle this, because as you said, the point of this example is to show that some safety checks happen at runtime.

Thank you for the review, madam (@carols10cents). If I may offer two alternatives instead, then...

  1. My initial idea was to use rand::random::<usize>() instead, so as to get a "random" value. But the tests really didn't want to function with external crates installed.
  2. A few days ago, completely by chance, I came upon another, way simpler solution. Maybe that would be preferable? (Admittedly, I'd prefer it if this "solution" didn't work either, but, oh well...)

@carols10cents
Copy link
Member

  • My initial idea was to use rand::random::<usize>() instead, so as to get a "random" value. But the tests really didn't want to function with external crates installed.

Yeah, we want to avoid using external crates as much as possible.

  • A few days ago, completely by chance, I came upon another, way simpler solution. Maybe that would be preferable? (Admittedly, I'd prefer it if this "solution" didn't work either, but, oh well...)

I don't think we've explained const at this point in the book, so that won't work either.

I'm going to experiment with a solution when I have time.

@eest
Copy link

eest commented Dec 29, 2020

@carols10cents I was pointed here after asking about the same docs discrepancy on discord. For what it is worth, const has at least been mentioned on the previous page: https://doc.rust-lang.org/book/ch03-01-variables-and-mutability.html#differences-between-variables-and-constants if that helps.

@carols10cents
Copy link
Member

@eest const is not the only issue I have with the example suggested here

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 this pull request may close these issues.

3 participants