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

[Merged by Bors] - Add safe constructors for untyped pointers Ptr and PtrMut #6539

Closed
wants to merge 8 commits into from

Conversation

JoJoJet
Copy link
Member

@JoJoJet JoJoJet commented Nov 10, 2022

Objective

Currently, Ptr and PtrMut can only be constructed via unsafe code. This means that downgrading a reference to an untyped pointer is very cumbersome, despite being a very simple operation.

Solution

Define conversions for easily and safely constructing untyped pointers. This is the non-owned counterpart to OwningPtr::make.

Before:

let ptr = unsafe { PtrMut::new(NonNull::from(&mut value).cast()) };

After:

let ptr = PtrMut::from(&mut value);

@DJMcNab DJMcNab added the X-Controversial There is active debate or serious implications around merging this PR label Nov 10, 2022
@DJMcNab
Copy link
Member

DJMcNab commented Nov 10, 2022

This looks reasonable to me, but I've added S-Controversial so it doesn't get merged before @TheRawMeatball has given their input.

@james7132 james7132 added the C-Feature A new feature, making something new possible label Nov 10, 2022
@alice-i-cecile alice-i-cecile added D-Complex Quite challenging from either a design or technical perspective. Ask for help! and removed X-Controversial There is active debate or serious implications around merging this PR labels Nov 10, 2022
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

This looks correct, but yes, I want verification from one of the pointer wizards.

Copy link
Contributor

@jakobhellermann jakobhellermann 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 to me.

Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

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

Generally LGTM. Some small nits.

crates/bevy_ptr/src/lib.rs Show resolved Hide resolved
crates/bevy_ptr/src/lib.rs Show resolved Hide resolved
crates/bevy_ptr/src/lib.rs Show resolved Hide resolved
crates/bevy_ptr/src/lib.rs Show resolved Hide resolved
@james7132 james7132 added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Nov 12, 2022
crates/bevy_ptr/src/lib.rs Outdated Show resolved Hide resolved
@cart
Copy link
Member

cart commented Nov 14, 2022

bors r+

bors bot pushed a commit that referenced this pull request Nov 14, 2022
# Objective

Currently, `Ptr` and `PtrMut` can only be constructed via unsafe code. This means that downgrading a reference to an untyped pointer is very cumbersome, despite being a very simple operation.

## Solution

Define conversions for easily and safely constructing untyped pointers. This is the non-owned counterpart to `OwningPtr::make`.

Before:

```rust
let ptr = unsafe { PtrMut::new(NonNull::from(&mut value).cast()) };
```

After:

```rust
let ptr = PtrMut::from(&mut value);
```


Co-authored-by: Carter Anderson <[email protected]>
@bors bors bot changed the title Add safe constructors for untyped pointers Ptr and PtrMut [Merged by Bors] - Add safe constructors for untyped pointers Ptr and PtrMut Nov 14, 2022
@bors bors bot closed this Nov 14, 2022
taiyoungjang pushed a commit to taiyoungjang/bevy that referenced this pull request Dec 15, 2022
…gine#6539)

# Objective

Currently, `Ptr` and `PtrMut` can only be constructed via unsafe code. This means that downgrading a reference to an untyped pointer is very cumbersome, despite being a very simple operation.

## Solution

Define conversions for easily and safely constructing untyped pointers. This is the non-owned counterpart to `OwningPtr::make`.

Before:

```rust
let ptr = unsafe { PtrMut::new(NonNull::from(&mut value).cast()) };
```

After:

```rust
let ptr = PtrMut::from(&mut value);
```


Co-authored-by: Carter Anderson <[email protected]>
alradish pushed a commit to alradish/bevy that referenced this pull request Jan 22, 2023
…gine#6539)

# Objective

Currently, `Ptr` and `PtrMut` can only be constructed via unsafe code. This means that downgrading a reference to an untyped pointer is very cumbersome, despite being a very simple operation.

## Solution

Define conversions for easily and safely constructing untyped pointers. This is the non-owned counterpart to `OwningPtr::make`.

Before:

```rust
let ptr = unsafe { PtrMut::new(NonNull::from(&mut value).cast()) };
```

After:

```rust
let ptr = PtrMut::from(&mut value);
```


Co-authored-by: Carter Anderson <[email protected]>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
…gine#6539)

# Objective

Currently, `Ptr` and `PtrMut` can only be constructed via unsafe code. This means that downgrading a reference to an untyped pointer is very cumbersome, despite being a very simple operation.

## Solution

Define conversions for easily and safely constructing untyped pointers. This is the non-owned counterpart to `OwningPtr::make`.

Before:

```rust
let ptr = unsafe { PtrMut::new(NonNull::from(&mut value).cast()) };
```

After:

```rust
let ptr = PtrMut::from(&mut value);
```


Co-authored-by: Carter Anderson <[email protected]>
@JoJoJet JoJoJet deleted the ptr-from branch September 20, 2023 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Feature A new feature, making something new possible D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants