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

Document std::os::raw. #46962

Merged
merged 4 commits into from
Feb 6, 2018
Merged

Document std::os::raw. #46962

merged 4 commits into from
Feb 6, 2018

Conversation

clarfonthey
Copy link
Contributor

This adds a brief explanation to each type and its definition according to C. This also helps clarify that the definitions of the types, as described by rustdoc, are not necessarily the same from platform to platform.

@rust-highfive
Copy link
Collaborator

r? @Kimundi

(rust_highfive has picked a reviewer for you, use r? to override)

@frewsxcv frewsxcv added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Dec 23, 2017
/// and `*mut c_void` is equivalent to C's `void*`. That said, this is
/// *not* the same as C's `void` return type, which is Rust's `()` type.
///
/// [pointer]: ../primitive.pointer.html
Copy link
Member

Choose a reason for hiding this comment

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

Broken link. Needs two ../ I guess.

[01:21:17] std/os/raw/index.html:66: broken link - std/os/primitive.pointer.html
[01:21:17] std/os/raw/enum.c_void.html:56: broken link - std/os/primitive.pointer.html
[01:21:26] thread 'main' panicked at 'found some broken links', /checkout/src/tools/linkchecker/main.rs:49:8

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 double-checked every link but this one. ><


[C's `char` type] is completely unlike [Rust's `char` type]; while Rust's type represents a unicode scalar value, C's `char` type is just an ordinary integer. In practice, this type will always be either [`i8`] or [`u8`], but you're technically not supposed to rely on this behaviour, as the standard only defines a char as being at least eight bits long.

C chars are most commonly used to make C strings. Unlike Rust, where the length of a string is included alongside the string, C strings mark the end of a string with a zero. See [`CStr`] for more information.
Copy link
Member

Choose a reason for hiding this comment

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

C strings mark the end of a string with a zero

This is slightly misleading as someone might think it means 0 the character, not the literal integer value 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would clarifying "the character '\0' " be sufficient?

@@ -0,0 +1,8 @@
Equivalent to C's `signed long` (`long`) type.

This type will usually be [`i64`], but is sometimes [`i32`] \(i.e. [`isize`]\) on 32-bit systems. Technically, the standard only requires that it be at least 32 bits, or at least the size of an [`int`].
Copy link
Member

Choose a reason for hiding this comment

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

Also note that it is equivalent to i32 even on 64bit Windows.

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 was not aware of this; I'll remove the remarks about isize and usize in these docs.

@kennytm kennytm added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Dec 23, 2017
@clarfonthey
Copy link
Contributor Author

Made revisions per comments

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 24, 2017
@alexcrichton
Copy link
Member

r? @QuietMisdreavus

@@ -0,0 +1,6 @@
Equivalent to C's `signed char` type.

This type will almost always be [`i8`], but its size is technically equal to the size of a C [`char`], which isn't very clear-cut.
Copy link
Member

Choose a reason for hiding this comment

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

The use of "isn't very clear-cut" throughout the number docs feels awkward. Is there any guidance we can paraphrase here? If not, i'd rather we use a phrase like "which is left under-specified" to make it feel marginally less wishy-washy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair; I was trying to avoid duplicating the docs too much, although it's mostly wishy-washy because C's definitions are incredibly wishy-washy.

I'll probably replace it with "which does not have a strict definition like Rust does" or something a bit more eloquent.

Copy link
Member

Choose a reason for hiding this comment

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

I don't see how C doesn't have a strict definition of size of signed char 😄. In C a char's size is defined to be 1 byte, always (although 1 byte is not defined to be 8 bits) (C11 §6.5.3.4/4).

Unless Rust is going to support non-8-bit-byte system, a signed char is always an i8.

@carols10cents carols10cents added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 9, 2018
@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 17, 2018
@kennytm
Copy link
Member

kennytm commented Jan 17, 2018

Hi @clarcharr, just checking if this PR is still actively worked on. Could you rephrase the sentence in #46962 (review)? Thanks!

@clarfonthey
Copy link
Contributor Author

@kennytm will do in a bit!

@shepmaster
Copy link
Member

Ping from triage, @clarcharr ! We haven't heard from you in a week; will you still be able to make the most recent set of changes?

@clarfonthey
Copy link
Contributor Author

@shepmaster + @kennytm + @QuietMisdreavus my laptop was out of commission for a bit and I'm working on rewording things right now.

@clarfonthey
Copy link
Contributor Author

I've pushed the latest version; let me know if things are okay!

@@ -0,0 +1,7 @@
Equivalent to C's `unsigned int` type.

This type will almost always be [`u16`], but may differ on some esoteric systems. The C standard technically only requires that this type be an unsigned integer with the same size as an [`int`]; some systems define it as a [`u16`], for example.
Copy link
Member

Choose a reason for hiding this comment

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

Is that first u16 supposed to be a u32?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

...yes

@pietroalbini pietroalbini added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 5, 2018
@pietroalbini
Copy link
Member

Ping from triage @QuietMisdreavus!

@QuietMisdreavus
Copy link
Member

I was still waiting on one last change, but it was small enough that i put it in myself. (Sorry @clarcharr! >_>)

@bors r+

@bors
Copy link
Contributor

bors commented Feb 6, 2018

📌 Commit fefd5e9 has been approved by QuietMisdreavus

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 6, 2018
@clarfonthey
Copy link
Contributor Author

@QuietMisdreavus I actually think I have the commit on my machine but I forgot to push ><

@kennytm
Copy link
Member

kennytm commented Feb 6, 2018

@bors rollup

kennytm added a commit to kennytm/rust that referenced this pull request Feb 6, 2018
…eavus

Document std::os::raw.

This adds a brief explanation to each type and its definition according to C. This also helps clarify that the definitions of the types, as described by rustdoc, are not necessarily the same from platform to platform.
bors added a commit that referenced this pull request Feb 6, 2018
Rollup of 7 pull requests

- Successful merges: #46962, #47986, #48012, #48013, #48026, #48031, #48036
- Failed merges:
@bors bors merged commit fefd5e9 into rust-lang:master Feb 6, 2018
@clarfonthey clarfonthey deleted the os_raw_docs branch April 15, 2018 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.