Skip to content

libstd/rustdoc: Document potential panics in RefCell where .borrow() is used implicitly in impls #47400

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Closed
Centril opened this issue Jan 12, 2018 · 0 comments · Fixed by #48365
Closed

libstd/rustdoc: Document potential panics in RefCell where .borrow() is used implicitly in impls #47400

Centril opened this issue Jan 12, 2018 · 0 comments · Fixed by #48365
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@Centril
Copy link
Contributor

Centril commented Jan 12, 2018

There are a bunch of implicit uses of .borrow(), which may panic if the RefCell is already mutably borrowed, inside impls of RefCell:

The potential panics should be noted with a header:

    /// # Panics
    ///
    /// Panics if the value is currently mutably borrowed. For a non-panicking variant, use
    /// [`try_borrow`](#method.try_borrow).

I'll file the PR fixing this if we agree that this is a problem in need of fixing.
@sfackler sfackler added the A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools label Jan 12, 2018
@Centril Centril added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Feb 16, 2018
Centril added a commit to Centril/rust that referenced this issue Feb 20, 2018
@Centril
RefCell: document panics in Clone, PartialEq, PartialOrd, Ord. Fixes r…
bde8555 
…ust-lang#47400
@Centril Centril mentioned this issue Feb 20, 2018
Merged
kennytm added a commit to kennytm/rust that referenced this issue Feb 28, 2018
@kennytm
Rollup merge of rust-lang#48365 - Centril:docs/document-refcell-panic…
c599463 
…s, r=frewsxcv

RefCell: document panics in Clone, PartialEq, PartialOrd, Ord.

This fixes rust-lang#47400 by adding:

```rust
    /// # Panics
    ///
    /// Panics if the value is currently mutably borrowed.
```
to said impls. They may panic since they call `.borrow()`.
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

RefCell: document panics in Clone, PartialEq, PartialOrd, Ord. Centril/rust
2 participants
@Centril @sfackler