Skip to content

Commit

Permalink
Merge #18
Browse files Browse the repository at this point in the history
18: Replace `unsafe_project` with safe `pin_projectable` attribute r=taiki-e a=Aaron1011

This PR replaces the `unsafe_project` attribute with a new, completely safe `pin_projectable` attribute. This attribtute enforces all of the guaranteees of pin projection:

* `#[repr(packed)]` types are disallowed completely, via a compile-time error.
* A `Drop` impl is unconditionally provided. A custom drop function can be provided by annotating a function with `#[pinned_drop]`. This function takes a `Pin<&mut MyType>`, which ensures that the user cannot move out of pinned fields in safe code.
* An `Unpin` impl is unconditionally provided. By default, this generates the same bounds as the (now removed) `Unpin` argument - all pin-projected fields are required to implement `Unpin`. A manual impl can be provided via the `unsafe_Unpin` attribute, and a impl of the new `unsafe` trait `UnsafeUnpin`.

This is a significant, non-backwards-compatible refactor of this crate. However, I believe it provides several significant benefits:

* Pin projection is now *a safe operation*. In the vast majority of cases, consumers of this crate can create pin projections without a single use of `unsafe`. This reduces the number of `unsafe` blocks that must be audited, and increases confidence that a crate does not trigger undefined behavior.
* The expressive power of the `#[unsafe_project]` and `#[pin_projectable]` is the same. Any code written using `#[unsafe_project]` can be adapted to `#[pin_projectable]`, even if relied in the `unsafe` nature of `#[unsafe_project]`:

  * `UnsafeUnpin` can be used to obtain complete control over the generated `Unpin` impl.
  * `Pin::get_unchecked_mut` can be used within a `#[pinned_drop]` function to obtain a `&mut MyStruct`, effectively turing `#[pinned_drop]` back into a regular `Drop` impl.
  * For `#[repr(packed)]` structs, there are two possible cases:
    * Pin projection is never used - no fields have the `#[pin]` attribute, or `project()` is never called on the base struct. In this case, using this crate for the struct is completely pointless, and the `#[unsafe_project]` attribute can be removed.
    * Pin projection *is* used. This is immediate undefined behavior - the new `#[pin_projectable`]` attribute is simply not allowing you to write broken code.
* Anything with the potential for undefined behavior now requires usage of the `unsafe` keyword, whearas the previous `#[unsafe_project]` attribute only required typing the word 'unsafe' in an argument. Using the actual `unsafe` keyword allows for proper integration with the `unsafe_code` lint, and tools [cargo geiger](https://github.com/anderejd/cargo-geiger). Note that the `unsafe_Unpin` argument still obeys this rule - the `UnsafeUnpin` trait it enables is unsafe, and failing to provide an impl of `UnsafeUnpin` is completely safe.

Unfortunately, this PR requires `pin-project` to be split into two crates - `pin-project` and `pin-project-internal`. This is due to the fact that `proc-macro` crates cannot currently export anything other than proc macros.  The crates are split as follows:
* A new `pin-project-internal` crates provides almost all of the functionality of this crate, with the exception of the `UnsafeUnpin` trait.
* The `pin-project` crate now re-exports everything from `pin-project-internal`, with added doc comments. It also provides the `UnsafeUnpin` trait.

Because the `pin-project-internal` crate must reference the `UnsafeUnpin` trait from `pin-project`, `pin-project-internal` implicitly depends on `pin-project`. To ensure that users can rename their dependency on `pin-project`, the crate [proc_macro_crate](https://crates.io/crates/proc_macro_crate) is used to dynamically determine the name of the `pin-project` crate at macro invocation time.

Due to several issues with Rustdoc's handling of procedural macros, the documentation for the `pin-project` crate will not currently render properly - however, all doctests will still run correctly. Once rust-lang/rust#62855 and rust-lang/rust#63048 are merged, the documentation will build correctly on nightly Rust.

@taiki-e: I'm happy to work with you to make any adjustments necessary to get this merged (once the referenced rustc PRs are merged).

Co-authored-by: Aaron Hill <aa1ronham@gmail.com>
  • Loading branch information
bors[bot] and Aaron1011 committed Aug 5, 2019
2 parents aaa486c + 60e4242 commit d6d5553
Show file tree
Hide file tree
Showing 27 changed files with 1,159 additions and 561 deletions.
38 changes: 5 additions & 33 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,34 +1,6 @@
[package]
name = "pin-project"
# NB: When modifying, also modify html_root_url in lib.rs
version = "0.3.4"
authors = ["Taiki Endo <te316e89@gmail.com>"]
edition = "2018"
license = "Apache-2.0/MIT"
description = "An attribute that creates a projection struct covering all the fields."
repository = "https://github.com/taiki-e/pin-project"
documentation = "https://docs.rs/pin-project/"
readme = "README.md"
keywords = ["pin", "macros", "attribute"]
categories = ["rust-patterns"]
exclude = ["/.travis.yml", "/bors.toml"]
[workspace]

[badges]
travis-ci = { repository = "taiki-e/pin-project" }

[lib]
proc-macro = true

[features]
# Default features.
default = ["project_attr"]
# Enable to use `project` attribute.
project_attr = ["syn/visit-mut"]

[dependencies]
proc-macro2 = "0.4.13"
quote = "0.6.8"
syn = { version = "0.15.29", features = ["full"] }

[dev-dependencies]
compiletest = { version = "0.3.21", package = "compiletest_rs", features = ["stable", "tmp"] }
members = [
"pin-project",
"pin-project-internal",
]
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ The current version of pin-project requires Rust 1.33 or later.

## Examples

[`unsafe_project`] attribute creates a projection struct covering all the fields.
[`pin_projectable`] attribute creates a projection struct covering all the fields.

```rust
use pin_project::unsafe_project;
use pin_project::pin_projectable;
use std::pin::Pin;

#[unsafe_project(Unpin)] // `(Unpin)` is optional (create the appropriate conditional Unpin implementation)
#[pin_projectable]
struct Foo<T, U> {
#[pin]
future: T,
Expand All @@ -61,7 +61,7 @@ impl<T, U> Foo<T, U> {

[Code like this will be generated](doc/struct-example-1.md)

[`unsafe_project`]: https://docs.rs/pin-project/0.3/pin_project/attr.unsafe_project.html
[`pin_projectable`]: https://docs.rs/pin-project/0.3/pin_project/attr.pin_projectable.html

## License

Expand Down
36 changes: 36 additions & 0 deletions pin-project-internal/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
[package]
name = "pin-project-internal"
# NB: When modifying, also modify html_root_url in lib.rs
version = "0.3.4"
authors = ["Taiki Endo <te316e89@gmail.com>"]
edition = "2018"
license = "Apache-2.0/MIT"
description = "An interal crate to support pin_project - do not use directly"
repository = "https://github.com/taiki-e/pin-project"
documentation = "https://docs.rs/pin-project/"
readme = "README.md"
keywords = ["pin", "macros", "attribute"]
categories = ["rust-patterns"]
exclude = ["/.travis.yml", "/bors.toml"]

[lib]
proc-macro = true

[features]
# Default features.
default = ["project_attr"]
# Enable to use `project` attribute.
project_attr = ["syn/visit-mut"]
# Enable to allow using the crate with a renamed 'pin-project' dependency
renamed = ["proc-macro-crate", "serde", "lazy_static"]


[dependencies]
proc-macro2 = "0.4.13"
quote = "0.6.8"
syn = { version = "0.15.29", features = ["full", "extra-traits"] }
proc-macro-crate = { version = "0.1.4", optional = true }
# Required until a new toml-rs release is made with https://github.com/alexcrichton/toml-rs/pull/311,
# and proc-macro-crate updates to that new version of toml-rs.
serde = { version = "1.0.97", optional = true }
lazy_static = { version = "1.3.0", optional = true }
File renamed without changes.
File renamed without changes.
50 changes: 50 additions & 0 deletions pin-project-internal/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#![recursion_limit = "256"]
#![doc(html_root_url = "https://docs.rs/pin-project/0.3.3")]
#![doc(test(attr(deny(warnings), allow(dead_code, unused_assignments, unused_variables))))]
#![warn(unsafe_code)]
#![warn(rust_2018_idioms, unreachable_pub)]
#![warn(single_use_lifetimes)]
#![warn(clippy::all, clippy::pedantic)]
#![warn(clippy::nursery)]

extern crate proc_macro;

#[macro_use]
mod utils;

mod pin_projectable;
#[cfg(feature = "project_attr")]
mod project;

use proc_macro::TokenStream;

#[cfg(feature = "project_attr")]
#[proc_macro_attribute]
pub fn project(args: TokenStream, input: TokenStream) -> TokenStream {
assert!(args.is_empty());
TokenStream::from(project::attribute(input.into()))
}

/// This is a doc comment from the defining crate!
#[proc_macro]
pub fn pin_project(input: TokenStream) -> TokenStream {
TokenStream::from(
pin_projectable::pin_project(input.into()).unwrap_or_else(|e| e.to_compile_error()),
)
}

#[proc_macro_attribute]
pub fn pin_projectable(args: TokenStream, input: TokenStream) -> TokenStream {
TokenStream::from(
pin_projectable::attribute(args.into(), input.into())
.unwrap_or_else(|e| e.to_compile_error()),
)
}

#[cfg(feature = "renamed")]
lazy_static::lazy_static! {
pub(crate) static ref PIN_PROJECT_CRATE: String = {
proc_macro_crate::crate_name("pin-project")
.expect("pin-project-internal was used without pin-project!")
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@ use crate::utils::{proj_ident, VecExt};

use super::*;

pub(super) fn parse(args: TokenStream, mut item: ItemEnum) -> Result<TokenStream> {
pub(super) fn parse(
args: TokenStream,
mut item: ItemEnum,
pinned_drop: Option<ItemFn>,
) -> Result<TokenStream> {
let impl_drop = ImplDrop::new(item.generics.clone(), pinned_drop)?;
let mut impl_unpin = ImplUnpin::new(args, &item.generics)?;

if item.variants.is_empty() {
Expand All @@ -31,6 +36,7 @@ pub(super) fn parse(args: TokenStream, mut item: ItemEnum) -> Result<TokenStream
enum #proj_ident #proj_generics #where_clause #proj_item_body
};

proj_items.extend(impl_drop.build(ident));
proj_items.extend(impl_unpin.build(ident));
proj_items.extend(quote! {
impl #impl_generics #ident #ty_generics #where_clause {
Expand Down
Loading

0 comments on commit d6d5553

Please sign in to comment.