Use &raw in A|Rc::as_ptr by CAD97 · Pull Request #73845 · rust-lang/rust

CAD97 · 2020-06-28T18:37:54Z

This PR uses &raw for offsetting *mut [A]RcInner<T> -> *mut T.

Additionally, this updates the implementation of Weak::as_ptr to support unsized T, though it does not yet relax the bounds of Weak::as_ptr/into_raw/from_raw to accept unsized T.

rust-highfive · 2020-06-28T18:37:56Z

r? @sfackler

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

sfackler · 2020-06-28T19:03:18Z

r? @RalfJung

src/liballoc/rc.rs

CAD97 · 2020-06-30T16:49:16Z

Addressed the review comments (and in sync.rs as well, of course).

(I really need to figure out why format-on-save is not agreeing with ./x.py fmt....)

src/liballoc/rc.rs

Co-Authored-By: Ralf Jung <post@ralfj.de>

src/liballoc/rc.rs

RalfJung · 2020-07-03T10:42:37Z

Awesome, thanks a lot for enduring all these rounds of review. :)
@bors r+

@ghost

…arth Rollup of 12 pull requests Successful merges: - rust-lang#73140 (Fallback to xml.etree.ElementTree) - rust-lang#73670 (Add `format_args_capture` feature) - rust-lang#73693 (Use exhaustive match in const_prop.rs) - rust-lang#73845 (Use &raw in A|Rc::as_ptr) - rust-lang#73861 (Create E0768) - rust-lang#73881 (Standardize bibliographic citations in rustc API docs) - rust-lang#73925 (Improve comments from rust-lang#72617, as suggested by RalfJung) - rust-lang#73949 ([mir-opt] Fix mis-optimization and other issues with the SimplifyArmIdentity pass) - rust-lang#73984 (Edit docs for rustc_data_structures::graph::scc) - rust-lang#73985 (Fix "getting started" link) - rust-lang#73997 (fix typo) - rust-lang#73999 (Bump mingw-check CI image from Ubuntu 16.04 to 18.04.) Failed merges: - rust-lang#74000 (add `lazy_normalization_consts` feature gate) r? @ghost

....to allow use with unsized T. This is a follow-up to rust-lang#73845, which did the impl work required to be able to relax these bounds.

@RalfJung

Allow Weak::as_ptr and friends for unsized T Relaxes `impl<T> Weak<T>` to `impl<T: ?Sized> Weak<T>` for the methods `rc::Weak::as_ptr`, `into_raw`, and `from_raw`. Follow-up to rust-lang#73845, which did most of the impl work to make these functions work for `T: ?Sized`. We still have to adjust the implementation of `Weak::from_raw` here, however, because I missed a use of `ptr.is_null()` previously. This check was necessary when `into`/`from_raw` were first implemented, as `into_raw` returned `ptr::null()` for dangling weak. However, we now just (wrapping) offset dangling weaks' pointers the same as nondangling weak, so the null check is no longer necessary (or even hit). (I can submit just 17a928f as a separate PR if desired.) As a nice side effect, moves the `fn is_dangling` definition closer to `Weak::new`, which creates the dangling weak. This technically stabilizes that "something like `align_of_val_raw`" is possible to do. However, I believe the part of the functionality required by these methods here -- specifically, getting the alignment of a pointee from a pointer where it may be dangling iff the pointee is `Sized` -- is uncontroversial enough to stabilize these methods without a way to implement them on stable Rust. r? @RalfJung, who reviewed rust-lang#73845. ATTN: This changes (relaxes) the (input) generic bounds on stable fn!

Allow Weak::as_ptr and friends for unsized T Relaxes `impl<T> Weak<T>` to `impl<T: ?Sized> Weak<T>` for the methods `rc::Weak::as_ptr`, `into_raw`, and `from_raw`. Follow-up to rust-lang#73845, which did most of the impl work to make these functions work for `T: ?Sized`. We still have to adjust the implementation of `Weak::from_raw` here, however, because I missed a use of `ptr.is_null()` previously. This check was necessary when `into`/`from_raw` were first implemented, as `into_raw` returned `ptr::null()` for dangling weak. However, we now just (wrapping) offset dangling weaks' pointers the same as nondangling weak, so the null check is no longer necessary (or even hit). (I can submit just 17a928f as a separate PR if desired.) As a nice side effect, moves the `fn is_dangling` definition closer to `Weak::new`, which creates the dangling weak. This technically stabilizes that "something like `align_of_val_raw`" is possible to do. However, I believe the part of the functionality required by these methods here -- specifically, getting the alignment of a pointee from a pointer where it may be dangling iff the pointee is `Sized` -- is uncontroversial enough to stabilize these methods without a way to implement them on stable Rust. r? `@RalfJung,` who reviewed rust-lang#73845. ATTN: This changes (relaxes) the (input) generic bounds on stable fn!

RalfJung · 2020-12-26T22:25:54Z

src/liballoc/sync.rs

@@ -2279,13 +2292,6 @@ unsafe fn data_offset<T: ?Sized>(ptr: *const T) -> isize {
    unsafe { data_offset_align(align_of_val(&*ptr)) }


oops this line should have been align_of_val_raw now, right?

But I doubt that that's enough to fix #80365.

Yes, it definitely should have been. I don't know how I missed this one 🙃

This was missed in rust-lang#73845 when switching to use the raw operators. Fixes rust-lang#80365

Use raw version of align_of in rc data_offset This was missed in rust-lang#73845 when switching to use the raw operators. Fixes rust-lang#80365

It is currently possible to create a dangling `Weak` to a DST by calling `Weak::new()` for a sized type, then doing an unsized coercion. Therefore, the comments are wrong. These comments were added in <rust-lang#73845>. As far as I can tell, the guarantee in the comment was only previously used in the `as_ptr` method. However, the current implementation of `as_ptr` no longer relies on this guarantee.

…ibraheemdev Remove incorrect comments in `Weak` It is currently possible to create a dangling `Weak` to a DST by calling `Weak::new()` for a sized type, then doing an unsized coercion. Therefore, the comments are wrong. These comments were added in <rust-lang#73845>. As far as I can tell, the guarantee in the comment was only previously used in the `as_ptr` method. However, the current implementation of `as_ptr` no longer relies on this guarantee.

Rollup merge of #142700 - theemathas:remove-weak-comment, r=ibraheemdev Remove incorrect comments in `Weak` It is currently possible to create a dangling `Weak` to a DST by calling `Weak::new()` for a sized type, then doing an unsized coercion. Therefore, the comments are wrong. These comments were added in <#73845>. As far as I can tell, the guarantee in the comment was only previously used in the `as_ptr` method. However, the current implementation of `as_ptr` no longer relies on this guarantee.

It is currently possible to create a dangling `Weak` to a DST by calling `Weak::new()` for a sized type, then doing an unsized coercion. Therefore, the comments are wrong. These comments were added in <rust-lang#73845>. As far as I can tell, the guarantee in the comment was only previously used in the `as_ptr` method. However, the current implementation of `as_ptr` no longer relies on this guarantee.

…ibraheemdev Remove incorrect comments in `Weak` It is currently possible to create a dangling `Weak` to a DST by calling `Weak::new()` for a sized type, then doing an unsized coercion. Therefore, the comments are wrong. These comments were added in <rust-lang#73845>. As far as I can tell, the guarantee in the comment was only previously used in the `as_ptr` method. However, the current implementation of `as_ptr` no longer relies on this guarantee.

CAD97 added 3 commits June 28, 2020 14:21

Use raw_ref_op in A|Rc::as_ptr

db539c6

Do not require ptr validity in rc::data_offset

e4bdf47

Use impl for Weak::as_ptr that works for unsized T

d8a9c61

rust-highfive assigned sfackler Jun 28, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 28, 2020

rust-highfive assigned RalfJung and unassigned sfackler Jun 28, 2020

RalfJung reviewed Jun 29, 2020

View reviewed changes

src/liballoc/rc.rs Outdated Show resolved Hide resolved

RalfJung reviewed Jun 29, 2020

View reviewed changes

src/liballoc/rc.rs Outdated Show resolved Hide resolved

RalfJung reviewed Jun 29, 2020

View reviewed changes

src/liballoc/rc.rs Show resolved Hide resolved

CAD97 force-pushed the weak-as-unsized-ptr branch from dce66a3 to b581b39 Compare June 30, 2020 16:51

CAD97 added 3 commits June 30, 2020 15:01

Clarify safety comment for A|Rc::as_ptr

fc3dc72

Fix invalid pointer deref in Weak::as_ptr

0aecf3c

Clarify when rc::data_offset is safe

aed88e1

CAD97 force-pushed the weak-as-unsized-ptr branch from b581b39 to aed88e1 Compare June 30, 2020 19:03

RalfJung reviewed Jul 1, 2020

View reviewed changes

src/liballoc/rc.rs Outdated Show resolved Hide resolved

RalfJung reviewed Jul 1, 2020

View reviewed changes

src/liballoc/rc.rs Outdated Show resolved Hide resolved

CAD97 and others added 2 commits July 1, 2020 15:05

Apply documentation review suggestions

ac40d1e

Co-Authored-By: Ralf Jung <post@ralfj.de>

Simplify Weak::as_ptr impl

98789ac

CAD97 force-pushed the weak-as-unsized-ptr branch from 31f8489 to 98789ac Compare July 1, 2020 19:06

RalfJung reviewed Jul 1, 2020

View reviewed changes

src/liballoc/rc.rs Show resolved Hide resolved

RalfJung reviewed Jul 1, 2020

View reviewed changes

src/liballoc/rc.rs Outdated Show resolved Hide resolved

Reclarify safety comments in Weak::as_ptr

7498cad

RalfJung reviewed Jul 2, 2020

View reviewed changes

src/liballoc/rc.rs Show resolved Hide resolved

Note Weak.ptr never dangles sooner

1b5ac57

CAD97 force-pushed the weak-as-unsized-ptr branch from 0db0dcb to 1b5ac57 Compare July 2, 2020 21:08

bors merged commit 9a659c5 into rust-lang:master Jul 4, 2020

CAD97 deleted the weak-as-unsized-ptr branch July 4, 2020 05:02

CAD97 added a commit to CAD97/rust that referenced this pull request Jul 4, 2020

Relax bounds of rc::Weak::as_ptr and friends

207c52d

....to allow use with unsized T. This is a follow-up to rust-lang#73845, which did the impl work required to be able to relax these bounds.

CAD97 mentioned this pull request Jul 4, 2020

?Sized bounds for rc::Weak::as_ptr and friends #74022

Closed

RalfJung mentioned this pull request Jul 8, 2020

Support T: ?Sized in Weak::new() #50513

Closed

CAD97 mentioned this pull request Jul 8, 2020

Allow Weak::as_ptr and friends for unsized T #74160

Merged

RalfJung mentioned this pull request Jul 11, 2020

Tracking issue for RFC 2582, &raw [mut | const] $place (raw_ref_op) #64490

Closed

3 tasks

RalfJung mentioned this pull request Jul 27, 2020

Tracking Issue for raw_ref_macros #73394

Closed

3 tasks

SNCPlay42 mentioned this pull request Dec 25, 2020

UB inside Weak::as_ptr #80365

Closed

RalfJung reviewed Dec 26, 2020

View reviewed changes

CAD97 mentioned this pull request Dec 26, 2020

Use raw version of align_of in rc data_offset #80398

Merged

CAD97 added a commit to CAD97/rust that referenced this pull request Dec 26, 2020

Use raw version of align_of in rc data_offset

eeed311

This was missed in rust-lang#73845 when switching to use the raw operators. Fixes rust-lang#80365

cuviper added this to the 1.46 milestone May 2, 2024

theemathas mentioned this pull request Jun 19, 2025

Remove incorrect comments in Weak #142700

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use &raw in A|Rc::as_ptr#73845

Use &raw in A|Rc::as_ptr#73845
bors merged 10 commits intorust-lang:masterfrom
CAD97:weak-as-unsized-ptr

CAD97 commented Jun 28, 2020

Uh oh!

rust-highfive commented Jun 28, 2020

Uh oh!

sfackler commented Jun 28, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CAD97 commented Jun 30, 2020 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RalfJung commented Jul 3, 2020

Uh oh!

RalfJung Dec 26, 2020

Uh oh!

CAD97 Dec 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

		@@ -2279,13 +2292,6 @@ unsafe fn data_offset<T: ?Sized>(ptr: *const T) -> isize {
		unsafe { data_offset_align(align_of_val(&*ptr)) }

Uh oh!

Conversation

CAD97 commented Jun 28, 2020

Uh oh!

rust-highfive commented Jun 28, 2020

Uh oh!

sfackler commented Jun 28, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CAD97 commented Jun 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RalfJung commented Jul 3, 2020

Uh oh!

RalfJung Dec 26, 2020

Choose a reason for hiding this comment

Uh oh!

CAD97 Dec 26, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

CAD97 commented Jun 30, 2020 •

edited

Loading