Skip to content
Closed
Changes from 1 commit
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
a458996
first draft of pointer to fields
RustyYato Jun 5, 2019
7eb7ea4
added to the drawbacks and later sections
RustyYato Jun 5, 2019
7ada603
expanded up reference level explanation
RustyYato Jun 5, 2019
6a9405f
updated formatting and added reference to InitPtr
RustyYato Jun 5, 2019
9b98d97
updated parts about field types, updated drawbacks
RustyYato Jun 5, 2019
b816798
added note about the `Copy` trait.
RustyYato Jun 5, 2019
0eb3d5c
added unsafe to `Field` trait
RustyYato Jun 6, 2019
9d82646
added unresolved question: minimizing proposal
RustyYato Jun 6, 2019
a1b545e
Added `?Sized` bounds
RustyYato Jun 6, 2019
8b5e4e2
added some details abot field types
RustyYato Jun 7, 2019
14fc49f
reworked the RFC's presentation
RustyYato Jun 7, 2019
d3cac45
fixed the pin-projection example in guid-level
RustyYato Jun 7, 2019
d001442
fixed typo
RustyYato Jun 7, 2019
d671633
removed `trait Project` and `trait PinProjectable`
RustyYato Jun 24, 2019
08d9ec2
added inverse projections
RustyYato Jun 24, 2019
00de06a
updated motivation
RustyYato Jun 24, 2019
43d559b
reason for removing `Project` and `PinProjectable`
RustyYato Jun 24, 2019
7c5161a
merge
RustyYato Jun 24, 2019
a83911c
moved from `std` to `core`
RustyYato Jun 24, 2019
6da5da2
Update text/0000-ptr-to-field.md
RustyYato Jun 25, 2019
97e2a7f
Update text/0000-ptr-to-field.md
RustyYato Jun 25, 2019
9d78b37
updated wording based off of @CAD97's suggestions
RustyYato Jun 25, 2019
f362e75
Merge branch 'ptr-to-field' of https://github.com/KrishnaSannasi/rfcs…
RustyYato Jun 25, 2019
dece0b1
editted note about pointer metadata
RustyYato Jun 25, 2019
2e77eef
Update text/0000-ptr-to-field.md
RustyYato Jun 25, 2019
e0e7e80
Apply suggestions from code review
RustyYato Jun 25, 2019
da3afb6
Some more changes from @CAD97's review
RustyYato Jun 25, 2019
21cf26b
added note about enum variants as types
RustyYato Jun 25, 2019
6b31b65
Merge branch 'ptr-to-field' of https://github.com/KrishnaSannasi/rfcs…
RustyYato Jun 25, 2019
00b4d4b
fixed inverse projecting UB examples
RustyYato Jun 25, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
moved from std to core
  • Loading branch information
RustyYato committed Jun 24, 2019
commit a83911cfcc11eae2a072009bc421235672a6b3bb
16 changes: 8 additions & 8 deletions text/0000-ptr-to-field.md
Original file line number Diff line number Diff line change
Expand Up @@ -163,15 +163,15 @@ The compiler can decide whether to actual generate a field type, this is to help

## `*const T`/`*mut T`

`project_unchecked` and `wrapping_project` will both live inside of `std::ptr`.
`project_unchecked` and `wrapping_project` will both live inside of `core::ptr`.

We need both `project_unchecked` and `wrapping_project` because there are some important optimization available inside of LLVM related to aliasing and escape analysis. In particular the LLVM `inbounds` assertion tells LLVM that a pointer offset stays within the same allocation and if the pointer is invalid or the offset does not stay within the same allocation it is considered UB. This behaviour is exposed via `project_unchecked`. This can be used by implementations of the `Project` trait for smart pointers that are always valid, like `&T` to enable better codegen. `wrapping_project` on the other hand will not assert `inbounds`, and will just wrap around if the pointer offset is larger than `usize::max_value()`. This safe defined behaviour, even if it is almost always a bug, unlike `project_unchecked` which is UB on invalid pointers or offsets.

This corresponds with `std::ptr::add` and `std::ptr::wrapping_add` in safety and behaviour. `project_unchecked` and `project` need to be implemented as intrinsics because there is no way to assert that the pointer metadata for fat pointers of `Field::Parent` and `Field::Type` will always match in general without some other compiler support. This is necessary to allow unsized types to be used transparently with this scheme.
This corresponds with `core::ptr::add` and `core::ptr::wrapping_add` in safety and behaviour. `project_unchecked` and `project` need to be implemented as intrinsics because there is no way to assert that the pointer metadata for fat pointers of `Field::Parent` and `Field::Type` will always match in general without some other compiler support. This is necessary to allow unsized types to be used transparently with this scheme.
Copy link

Choose a reason for hiding this comment

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

So wait, how does this interact with unsized types exactly? This needs to be spelled out.

Since this is specifically to give vocabulary to the direct manipulation of the pointers, I suspect that this should require that the field pointer's metadata is derivable from the parent metadata. Note that this doesn't require equivalent metadata: a theoretical pointer with two metadatas could be split into two child fat pointers with one or the other metadata.

The intrinsics live in core::intrinsics, of course.

Copy link
Author

@RustyYato RustyYato Jun 25, 2019

Choose a reason for hiding this comment

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

Note that this doesn't require equivalent metadata: a theoretical pointer with two metadatas could be split into two child fat pointers with one or the other metadata.

Actually, you can only have 1 unsized field on a type, and it must be the last field, so the metadata will be exactly the same.

Also, I haven't considered how custom DST will interact with this, but that still hasn't been accepted. I will add that as an unanswered question.

The intrinsics live in core::intrinsics, of course.

Yes, I will make it clear where everything lives in the next revision. I see now that it wasn't clear enough


`inverse_project_unchecked` and `inverse_wrapping_project` are just like their counterparts in safety. `inverse_project_unchecked` is UB to use on invalid pointers, where `inverse_wrapping_project` just wraps around on overflow. But there is one important safety check that must be performed before dereferencing the resulting pointer. The resulting pointer may not actually point to a valid `*const F::Parent` if `*const F::Type` does not live inside of a `F::Parent`, so one must take care to ensure that the parent pointer is indeed valid. This is different from `project_unchecked` and `wrapping_project` because there one only needs to validate the original pointer, not the resulting pointer.

`inverse_project_unchecked` and `inverse_wrapping_project` correspond to `std::ptr::sub` and `std::ptr::wrapping_sub` in safety and behaviour. They also must be implemented as intrinsics for the same reasons as stated above.
`inverse_project_unchecked` and `inverse_wrapping_project` correspond to `core::ptr::sub` and `core::ptr::wrapping_sub` in safety and behaviour. They also must be implemented as intrinsics for the same reasons as stated above.

For example of where `project_unchecked` would be UB.

Expand All @@ -194,7 +194,7 @@ If the raw pointer is valid, then the result of both `project_unchecked` and `wr

## Type and Traits

The `Field` trait and the `FieldDescriptor` type will live inside the `std::project` module, and will not be added to `prelude`.
The `Field` trait and the `FieldDescriptor` type will live inside the `core::project` module, and will not be added to `prelude`.

The `Field` trait will only be implemented by the compiler, and it compiler should make sure that no other implementations exist. This allows unsafe code to assume that the implmentors or the `Field` trait always refer to valid fields. The `FieldDescriptor` type may be unnecessary raw pointer projections are implemented via intrinsics, if so we can remove it entirely.

Expand Down Expand Up @@ -297,7 +297,7 @@ unsafe impl PinProjectable for Foo.bar {}
/// main.rs

fn main() {
use std::project::Project;
use core::project::Project;
use foo::Foo;

// These type annotations are unnecessary, but I put them in for clarity
Expand All @@ -317,8 +317,8 @@ We still need some `unsafe` to tell that it is indeed safe to project through th

## Reference-level explanation (`Project` and `PinProjectable`)

The `Project` trait will live inside the `std::project` module, and will not be added to `prelude`.
The `PinProjectable` trait will be added to `std::marker`, and will also not be added to `prelude`.
The `Project` trait will live inside the `core::project` module, and will not be added to `prelude`.
The `PinProjectable` trait will be added to `core::marker`, and will also not be added to `prelude`.

The `Project` trait will be implemented for `*const T`, `*mut T`, `&T`, `&mut T`, `Pin<&T>`, `Pin<&mut T>`. Other smart pointers can get implementations later if they need them. We may also provide the following implementations to allow better documentation of intent

Expand All @@ -331,7 +331,7 @@ Because the following impls conflict,
```rust
unsafe impl<F: Field> PinProjectable for F where F::Parent: Unpin {}

struct Foo(std::marker::PhantomPinned);
struct Foo(core::marker::PhantomPinned);

unsafe impl PinProjectable for Foo.0 {}
```
Expand Down