Skip to content

Add some more commentary to FFI tutorial. #10822

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

Closed
wants to merge 1 commit into from

Conversation

ezyang
Copy link
Contributor

@ezyang ezyang commented Dec 5, 2013

Signed-off-by: Edward Z. Yang [email protected]

@@ -206,12 +215,17 @@ impl<T: Send> Unique<T> {
}
}

// The key ingredient for safety, we associate a destructor with
// Unique<T>, making the struct manage the raw pointer.
Copy link
Member

Choose a reason for hiding this comment

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

Is it worth being specific about what sort of management is happening? "making the struct free the raw pointer when the struct goes out of scope", maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SGTM.

@ezyang
Copy link
Contributor Author

ezyang commented Dec 5, 2013

Let me know if you want me to rebase these into one commit.

@thestinger
Copy link
Contributor

@ezyang: one commit would be nicer imo, but it's not incredibly important

bors added a commit that referenced this pull request Dec 10, 2013
@bors bors closed this Dec 10, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 2, 2023
Ignore `#[cfg]`'d out code in `needless_else`

changelog: none (same release as rust-lang#10810)

`#[cfg]` making things fun once more

This lead me to think about macro calls that expand to nothing as well, but apparently they produce an empty stmt in the AST so are already handled, added a test for that

r? `@llogiq`
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.

5 participants