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 safety of Path casting #67635

Merged
merged 1 commit into from
Dec 28, 2019
Merged

Conversation

Mark-Simulacrum
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum commented Dec 26, 2019

I would personally feel more comfortable making the relevant (internal anyway) types repr(transparent) and then documenting that we can make these casts because of that, but I believe this is a more minimal PR, so posting it first.

Resolves #45910.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 26, 2019
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Looks good!

I guess the comments from #61969 are still in effect and we are waiting on "attribute privacy" before adding real #[repr(transparent)] attributes here.

Personally I don't consider that important and would be willing to add #[repr(transparent)] today; that attribute doesn't give anyone outside std the right to make assumptions about what the internal type is, as the visibility of the field of OsStr is private to std. That is, adding #[repr(transparent)] doesn't make it valid for code outside of std to start pointer cast between OsStr and [u8].

@dtolnay
Copy link
Member

dtolnay commented Dec 26, 2019

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 26, 2019

📌 Commit 9c0f3f7 has been approved by dtolnay

@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 Dec 26, 2019
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Dec 26, 2019
…dtolnay

Document safety of Path casting

I would personally feel more comfortable making the relevant (internal anyway) types repr(transparent) and then documenting that we can make these casts because of that, but I believe this is a more minimal PR, so posting it first.

Resolves rust-lang#45910.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Dec 26, 2019
…dtolnay

Document safety of Path casting

I would personally feel more comfortable making the relevant (internal anyway) types repr(transparent) and then documenting that we can make these casts because of that, but I believe this is a more minimal PR, so posting it first.

Resolves rust-lang#45910.
bors added a commit that referenced this pull request Dec 26, 2019
Rollup of 12 pull requests

Successful merges:

 - #67112 (Refactor expression parsing thoroughly)
 - #67192 (Various const eval and pattern matching ICE fixes)
 - #67287 (typeck: note other end-point when checking range pats)
 - #67459 (prune ill-conceived BTreeMap iter_mut assertion and test its mutability)
 - #67576 (reuse `capacity` variable in slice::repeat)
 - #67602 (Use issue = "none" instead of "0" in intrinsics)
 - #67614 (Set callbacks globally)
 - #67617 (Remove `compiler_builtins_lib` documentation)
 - #67629 (Remove redundant link texts)
 - #67632 (Convert collapsed to shortcut reference links)
 - #67633 (Update .mailmap)
 - #67635 (Document safety of Path casting)

Failed merges:

r? @ghost
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Dec 27, 2019
…dtolnay

Document safety of Path casting

I would personally feel more comfortable making the relevant (internal anyway) types repr(transparent) and then documenting that we can make these casts because of that, but I believe this is a more minimal PR, so posting it first.

Resolves rust-lang#45910.
oli-obk added a commit to oli-obk/rust that referenced this pull request Dec 27, 2019
…dtolnay

Document safety of Path casting

I would personally feel more comfortable making the relevant (internal anyway) types repr(transparent) and then documenting that we can make these casts because of that, but I believe this is a more minimal PR, so posting it first.

Resolves rust-lang#45910.
bors added a commit that referenced this pull request Dec 28, 2019
Rollup of 15 pull requests

Successful merges:

 - #65244 (add IntoFuture trait and support for await)
 - #67576 (reuse `capacity` variable in slice::repeat)
 - #67588 (Use NonNull in slice::Iter and slice::IterMut.)
 - #67594 (Update libc to 0.2.66)
 - #67602 (Use issue = "none" instead of "0" in intrinsics)
 - #67604 (Add Scalar::to_(u|i)16 methods)
 - #67617 (Remove `compiler_builtins_lib` documentation)
 - #67621 (Use the correct type for static qualifs)
 - #67629 (Remove redundant link texts)
 - #67632 (Convert collapsed to shortcut reference links)
 - #67633 (Update .mailmap)
 - #67635 (Document safety of Path casting)
 - #67654 (Add regression test for old NLL ICE)
 - #67659 (Stabilize the `matches!` macro)
 - #67664 (Fix some mailmap entries)

Failed merges:

r? @ghost
@bors bors merged commit 9c0f3f7 into rust-lang:master Dec 28, 2019
@Mark-Simulacrum Mark-Simulacrum deleted the path-doc-unsafe branch January 2, 2020 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

Document pointer cast safety in std::path
4 participants