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

Tweak std::marker docs #36424

Merged
merged 1 commit into from
Sep 16, 2016
Merged

Conversation

kmcallister
Copy link
Contributor

@@ -60,13 +76,16 @@ pub trait Sized {
}

/// Types that can be "unsized" to a dynamically sized type.
///
/// For example, the sized array type `[i8; 2]` implements `Unsize<[u8]>` and
Copy link
Member

Choose a reason for hiding this comment

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

It might be worth briefly talking about how Unsize is paired with CoerceUnsized and what it's used for here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's something I don't understand particularly well. Do you have a reference?

Copy link
Member

Choose a reason for hiding this comment

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

I think the RFC should still mostly reflect the current reality but I'm not super familiar with DST coercions either: https://github.com/nrc/rfcs/blob/4b99006cac67f7ad89c670bcb351d32bc0d5d33f/text/0000-dst-coercion.md

@frewsxcv
Copy link
Member

Linkcheck stage2 (x86_64-unknown-linux-gnu)
core/marker/trait.Sync.html:98: broken link - core/mem/fn.transmute.html
thread 'main' panicked at 'found some broken links', src/tools/linkchecker/main.rs:54

/// [rwlock]: ../../std/sync/struct.RwLock.html
/// [unsafecell]: ../cell/struct.UnsafeCell.html
/// [ub]: ../../reference.html#behavior-considered-undefined
/// [transmute]: ../mem/fn.transmute.html
Copy link
Member

Choose a reason for hiding this comment

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

core::mem::transmute doesn't exist, which might make this problematic?

Copy link
Member

Choose a reason for hiding this comment

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

Add "../std".

@kmcallister
Copy link
Contributor Author

Addressed review comments. Thanks everyone.

@GuillaumeGomez
Copy link
Member

Please squash your commits! Once done, I'd prefer to wait that #36363 has been merged before r+ yours to avoid conflicts.

@kmcallister
Copy link
Contributor Author

Yeah, that's going to conflict. I'll fix my PR once it lands.

@bors
Copy link
Contributor

bors commented Sep 14, 2016

☔ The latest upstream changes (presumably #36472) made this pull request unmergeable. Please resolve the merge conflicts.

@kmcallister kmcallister force-pushed the gh-29361-marker-docs branch 2 times, most recently from 58fafb7 to 81d3f6a Compare September 15, 2016 00:23
@kmcallister
Copy link
Contributor Author

I resolved conflicts and rebased.

/// [typeid]: ../any/struct.TypeId.html
/// [param]: http://en.wikipedia.org/wiki/Parametricity
/// [size_of]: ../mem/fn.size_of.html
///
Copy link
Member

Choose a reason for hiding this comment

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

This line is useless.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

///
/// If your struct does not in fact *own* the data of type `T`, it is
/// better to use a reference type, like `PhantomData<&'a T>`
/// (ideally) or `PhantomData<*const T>` (if no lifetime applies), so
/// as not to indicate ownership.
///
/// [drop check]: ../../nomicon/dropck.html
///
Copy link
Member

Choose a reason for hiding this comment

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

This line is useless.

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 saw that some of the existing docs have it. I think it makes things more readable in the source code. If you feel very strongly I'll remove it, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@GuillaumeGomez
Copy link
Member

Thanks!

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Sep 15, 2016

📌 Commit b735c1b has been approved by GuillaumeGomez

sophiajt pushed a commit to sophiajt/rust that referenced this pull request Sep 15, 2016
…r-docs, r=GuillaumeGomez

Tweak std::marker docs

Fixes rust-lang#29361.

r? @GuillaumeGomez
sophiajt pushed a commit to sophiajt/rust that referenced this pull request Sep 16, 2016
…r-docs, r=GuillaumeGomez

Tweak std::marker docs

Fixes rust-lang#29361.

r? @GuillaumeGomez
bors added a commit that referenced this pull request Sep 16, 2016
Rollup of 8 pull requests

- Successful merges: #36383, #36424, #36480, #36484, #36505, #36509, #36519, #36521
- Failed merges:
@bors bors merged commit b735c1b into rust-lang:master Sep 16, 2016
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.

6 participants