Skip to content

Misc API doc improvements#14304

Merged
bors merged 7 commits intorust-lang:masterfrom
brson:moredocs
May 20, 2014
Merged

Misc API doc improvements#14304
bors merged 7 commits intorust-lang:masterfrom
brson:moredocs

Conversation

@brson
Copy link
Contributor

@brson brson commented May 20, 2014

Includes module docs for cell.

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
Contributor

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