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

Misc API doc improvements #14304

Merged
merged 7 commits into from
May 20, 2014
Merged

Misc API doc improvements #14304

merged 7 commits into from
May 20, 2014

Conversation

brson
Copy link
Contributor

@brson brson commented May 20, 2014

Includes module docs for cell.

//! impl Graph {
//! fn minimum_spanning_tree(&self) -> Vec<(uint, uint)> {
//! // Create a new scope to contain the lifetime of the
//! // dynamic borrow
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to encourage using scopes to contain RefCell borrows, or is it better to encourage the use of drop() on the borrowed Ref? We need scopes for &mut, but drop() is sufficient for Ref.

Copy link
Member

Choose a reason for hiding this comment

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

I like the visual aspect of the new scope.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps, but there are definitely cases where drop() is more useful. The question is, do we want to suggest to readers that the scope is necessary here, like it is with &mut, or do we want to show them that drop() works?

I'm ok with either answer, I just want to make sure it's an intentional choice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't care strongly but I'll rationalize this choice as: blocks are the more fundamental way for handling value lifetimes; they are built into the language, so they are more appropriate for demonstrating basic concepts.

@lilyball
Copy link
Contributor

Overall this is pretty good. r=me with comments.

bors added a commit that referenced this pull request May 20, 2014
Includes module docs for `cell`.
@bors bors closed this May 20, 2014
@bors bors merged commit c9ab33a into rust-lang:master May 20, 2014
flip1995 pushed a commit to flip1995/rust that referenced this pull request Feb 27, 2025
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