Skip to content

Document bounds checking in the book #30310

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

Merged
merged 1 commit into from
Dec 13, 2015
Merged

Conversation

mbrubeck
Copy link
Contributor

r? @steveklabnik

Currently neither the API docs nor the book clearly explain that out-of-bounds array indexing causes a panic. Since this is fairly important and seems to surprise a number of new Rust programmers, I think it's worth adding to both places. (But if you think it would be better to put this info in the API docs only, that's fine too.)

Some specific things I'd like feedback on:

  • The new text here talks about panicking, which hasn't been formally introduced at this point in chapter 5 (though it has been mentioned in previous sections too).
  • Similarly the Vec::get example uses Option<T> which hasn't been fully introduced yet. Should we leave out this example?

@mbrubeck
Copy link
Contributor Author

(Note: This PR doesn't add anything to the API docs yet; I'd like to do that in a follow-up issue after we work out what to put in the book.)

println!("Item 7 is {}", v[7]);
```

then the current thread will [panic][] with a message like this:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need the extra []s here

@steveklabnik
Copy link
Member

The new text here talks about panicking, which hasn't been formally introduced at this point

normally, this would be a big issue, but since we're doing a reorganization soon, it's cool.

r=me with the nit

@mbrubeck
Copy link
Contributor Author

Fixed the nit.

@steveklabnik
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Collaborator

bors commented Dec 11, 2015

📌 Commit 5b9dd6a has been approved by steveklabnik

@bors
Copy link
Collaborator

bors commented Dec 12, 2015

⌛ Testing commit 5b9dd6a with merge 8a68364...

@bors
Copy link
Collaborator

bors commented Dec 12, 2015

💔 Test failed - auto-mac-64-opt

@alexcrichton
Copy link
Member

@bors: retry

On Sat, Dec 12, 2015 at 12:00 AM, bors [email protected] wrote:

[image: 💔] Test failed - auto-mac-64-opt
http://buildbot.rust-lang.org/builders/auto-mac-64-opt/builds/7367


Reply to this email directly or view it on GitHub
#30310 (comment).

@bors
Copy link
Collaborator

bors commented Dec 13, 2015

⌛ Testing commit 5b9dd6a with merge 35b6461...

bors added a commit that referenced this pull request Dec 13, 2015
r? @steveklabnik

Currently neither the API docs nor the book clearly explain that out-of-bounds array indexing causes a panic.  Since this is fairly important and seems to surprise a number of new Rust programmers, I think it's worth adding to both places.  (But if you think it would be better to put this info in the API docs only, that's fine too.)

Some specific things I'd like feedback on:

* The new text here talks about panicking, which hasn't been formally introduced at this point in chapter 5 (though it has been mentioned in previous sections too).
* Similarly the `Vec::get` example uses `Option<T>` which hasn't been fully introduced yet.  Should we leave out this example?
@bors bors merged commit 5b9dd6a into rust-lang:master Dec 13, 2015
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.

4 participants