Skip to content

Commit

Permalink
Merge #321
Browse files Browse the repository at this point in the history
321: Prepare for removal of safe_packed_borrows lint r=taiki-e a=taiki-e

Same as taiki-e/pin-project-lite#55, but, pin-project's safety no longer depends on this, so there is no need to rush release. 

Co-authored-by: Taiki Endo <te316e89@gmail.com>
  • Loading branch information
bors[bot] and taiki-e authored Mar 27, 2021
2 parents a51d39f + c9be8c9 commit 6539cfd
Show file tree
Hide file tree
Showing 75 changed files with 319 additions and 226 deletions.
2 changes: 1 addition & 1 deletion examples/not_unpin-expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ const _: () = {
//
// See ./struct-default-expanded.rs and /~https://github.com/taiki-e/pin-project/pull/34
// for details.
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &Struct<T, U>) {
let _ = &this.pinned;
let _ = &this.unpinned;
Expand Down
2 changes: 1 addition & 1 deletion examples/pinned_drop-expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ const _: () = {
//
// See ./struct-default-expanded.rs and /~https://github.com/taiki-e/pin-project/pull/34
// for details.
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<'a, T>(this: &Struct<'a, T>) {
let _ = &this.was_dropped;
let _ = &this.field;
Expand Down
2 changes: 1 addition & 1 deletion examples/project_replace-expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ const _: () = {
//
// See ./struct-default-expanded.rs and /~https://github.com/taiki-e/pin-project/pull/34
// for details.
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &Struct<T, U>) {
let _ = &this.pinned;
let _ = &this.unpinned;
Expand Down
8 changes: 3 additions & 5 deletions examples/struct-default-expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,16 @@ const _: () = {
// Ensure that it's impossible to use pin projections on a #[repr(packed)]
// struct.
//
// Taking a reference to a packed field is unsafe, and applying
// #[forbid(safe_packed_borrows)] makes sure that doing this without
// an 'unsafe' block (which we deliberately do not generate)
// is a hard error.
// Taking a reference to a packed field is UB, and applying
// `#[forbid(unaligned_references)]` makes sure that doing this is a hard error.
//
// If the struct ends up having #[repr(packed)] applied somehow,
// this will generate an (unfriendly) error message. Under all reasonable
// circumstances, we'll detect the #[repr(packed)] attribute, and generate
// a much nicer error above.
//
// See /~https://github.com/taiki-e/pin-project/pull/34 for more details.
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &Struct<T, U>) {
let _ = &this.pinned;
let _ = &this.unpinned;
Expand Down
2 changes: 1 addition & 1 deletion examples/unsafe_unpin-expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ const _: () = {
//
// See ./struct-default-expanded.rs and /~https://github.com/taiki-e/pin-project/pull/34
// for details.
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &Struct<T, U>) {
let _ = &this.pinned;
let _ = &this.unpinned;
Expand Down
30 changes: 20 additions & 10 deletions pin-project-internal/src/pin_project/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -996,7 +996,7 @@ fn make_proj_impl(
/// This currently does two checks:
/// * Checks the attributes of structs to ensure there is no `[repr(packed)]`.
/// * Generates a function that borrows fields without an unsafe block and
/// forbidding `safe_packed_borrows` lint.
/// forbidding `unaligned_references` lint.
fn ensure_not_packed(orig: &OriginalType<'_>, fields: &Fields) -> Result<TokenStream> {
for meta in orig.attrs.iter().filter_map(|attr| attr.parse_meta().ok()) {
if let Meta::List(list) = meta {
Expand All @@ -1019,17 +1019,14 @@ fn ensure_not_packed(orig: &OriginalType<'_>, fields: &Fields) -> Result<TokenSt
}
}

// As proc-macro-derive can't rewrite the structure definition,
// it's probably no longer necessary, but it keeps it for now.

// Workaround for /~https://github.com/taiki-e/pin-project/issues/32
// Through the tricky use of proc macros, it's possible to bypass
// the above check for the `repr` attribute.
// To ensure that it's impossible to use pin projections on a `#[repr(packed)]`
// struct, we generate code like this:
//
// ```rust
// #[forbid(safe_packed_borrows)]
// #[forbid(unaligned_references)]
// fn assert_not_repr_packed(val: &MyStruct) {
// let _field1 = &val.field1;
// let _field2 = &val.field2;
Expand All @@ -1038,10 +1035,8 @@ fn ensure_not_packed(orig: &OriginalType<'_>, fields: &Fields) -> Result<TokenSt
// }
// ```
//
// Taking a reference to a packed field is unsafe, and applying
// `#[forbid(safe_packed_borrows)]` makes sure that doing this without
// an `unsafe` block (which we deliberately do not generate)
// is a hard error.
// Taking a reference to a packed field is UB, and applying
// `#[forbid(unaligned_references)]` makes sure that doing this is a hard error.
//
// If the struct ends up having `#[repr(packed)]` applied somehow,
// this will generate an (unfriendly) error message. Under all reasonable
Expand All @@ -1061,6 +1056,21 @@ fn ensure_not_packed(orig: &OriginalType<'_>, fields: &Fields) -> Result<TokenSt
// `#[repr(packed)]` in the first place.
//
// See also /~https://github.com/taiki-e/pin-project/pull/34.
//
// Note:
// - pin-project v0.4.3 or later (#135, v0.4.0-v0.4.2 are already yanked for
// another reason) is internally proc-macro-derive, so they are not
// affected by the problem that the struct definition is rewritten by
// another macro after the #[pin_project] is expanded.
// So this is probably no longer necessary, but it keeps it for now.
//
// - Lint-based tricks aren't perfect, but they're much better than nothing:
// /~https://github.com/taiki-e/pin-project-lite/issues/26
//
// - Enable both unaligned_references and safe_packed_borrows lints
// because unaligned_references lint does not exist in older compilers:
// /~https://github.com/taiki-e/pin-project-lite/pull/55
// /~https://github.com/rust-lang/rust/pull/82525
let mut field_refs = vec![];
match fields {
Fields::Named(FieldsNamed { named, .. }) => {
Expand All @@ -1080,7 +1090,7 @@ fn ensure_not_packed(orig: &OriginalType<'_>, fields: &Fields) -> Result<TokenSt
let (impl_generics, ty_generics, where_clause) = orig.generics.split_for_impl();
let ident = orig.ident;
Ok(quote! {
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed #impl_generics (this: &#ident #ty_generics) #where_clause {
#(let _ = #field_refs;)*
}
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/default/struct.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &Struct<T, U>) {
let _ = &this.pinned;
let _ = &this.unpinned;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/default/tuple_struct.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &TupleStruct<T, U>) {
let _ = &this.0;
let _ = &this.1;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/multifields/struct.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &Struct<T, U>) {
let _ = &this.pinned1;
let _ = &this.pinned2;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/multifields/tuple_struct.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &TupleStruct<T, U>) {
let _ = &this.0;
let _ = &this.1;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/naming/struct-all.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &Struct<T, U>) {
let _ = &this.pinned;
let _ = &this.unpinned;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/naming/struct-mut.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &Struct<T, U>) {
let _ = &this.pinned;
let _ = &this.unpinned;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/naming/struct-none.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &Struct<T, U>) {
let _ = &this.pinned;
let _ = &this.unpinned;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/naming/struct-own.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &Struct<T, U>) {
let _ = &this.pinned;
let _ = &this.unpinned;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/naming/struct-ref.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &Struct<T, U>) {
let _ = &this.pinned;
let _ = &this.unpinned;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/naming/tuple_struct-all.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &TupleStruct<T, U>) {
let _ = &this.0;
let _ = &this.1;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/naming/tuple_struct-mut.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &TupleStruct<T, U>) {
let _ = &this.0;
let _ = &this.1;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/naming/tuple_struct-none.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &TupleStruct<T, U>) {
let _ = &this.0;
let _ = &this.1;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/naming/tuple_struct-own.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &TupleStruct<T, U>) {
let _ = &this.0;
let _ = &this.1;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/naming/tuple_struct-ref.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &TupleStruct<T, U>) {
let _ = &this.0;
let _ = &this.1;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/not_unpin/struct.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &Struct<T, U>) {
let _ = &this.pinned;
let _ = &this.unpinned;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/not_unpin/tuple_struct.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &TupleStruct<T, U>) {
let _ = &this.0;
let _ = &this.1;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/pinned_drop/enum.expanded.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use pin_project::{pin_project, pinned_drop};
use std::pin::Pin;
use pin_project::{pin_project, pinned_drop};
# [pin (__private (PinnedDrop , project = EnumProj , project_ref = EnumProjRef))]
enum Enum<T, U> {
Struct {
Expand Down
3 changes: 2 additions & 1 deletion tests/expand/pinned_drop/enum.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use pin_project::{pin_project, pinned_drop};
use std::pin::Pin;

use pin_project::{pin_project, pinned_drop};

#[pin_project(PinnedDrop, project = EnumProj, project_ref = EnumProjRef)]
enum Enum<T, U> {
Struct {
Expand Down
4 changes: 2 additions & 2 deletions tests/expand/pinned_drop/struct.expanded.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use pin_project::{pin_project, pinned_drop};
use std::pin::Pin;
use pin_project::{pin_project, pinned_drop};
#[pin(__private(PinnedDrop))]
struct Struct<T, U> {
#[pin]
Expand Down Expand Up @@ -78,7 +78,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &Struct<T, U>) {
let _ = &this.pinned;
let _ = &this.unpinned;
Expand Down
3 changes: 2 additions & 1 deletion tests/expand/pinned_drop/struct.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use pin_project::{pin_project, pinned_drop};
use std::pin::Pin;

use pin_project::{pin_project, pinned_drop};

#[pin_project(PinnedDrop)]
struct Struct<T, U> {
#[pin]
Expand Down
4 changes: 2 additions & 2 deletions tests/expand/pinned_drop/tuple_struct.expanded.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use pin_project::{pin_project, pinned_drop};
use std::pin::Pin;
use pin_project::{pin_project, pinned_drop};
#[pin(__private(PinnedDrop))]
struct TupleStruct<T, U>(#[pin] T, U);
#[allow(box_pointers)]
Expand Down Expand Up @@ -66,7 +66,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &TupleStruct<T, U>) {
let _ = &this.0;
let _ = &this.1;
Expand Down
3 changes: 2 additions & 1 deletion tests/expand/pinned_drop/tuple_struct.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use pin_project::{pin_project, pinned_drop};
use std::pin::Pin;

use pin_project::{pin_project, pinned_drop};

#[pin_project(PinnedDrop)]
struct TupleStruct<T, U>(#[pin] T, U);

Expand Down
2 changes: 1 addition & 1 deletion tests/expand/project_replace/struct.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &Struct<T, U>) {
let _ = &this.pinned;
let _ = &this.unpinned;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/project_replace/tuple_struct.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &TupleStruct<T, U>) {
let _ = &this.0;
let _ = &this.1;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/pub/struct.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &Struct<T, U>) {
let _ = &this.pinned;
let _ = &this.unpinned;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/pub/tuple_struct.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &TupleStruct<T, U>) {
let _ = &this.0;
let _ = &this.1;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/unsafe_unpin/struct.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &Struct<T, U>) {
let _ = &this.pinned;
let _ = &this.unpinned;
Expand Down
2 changes: 1 addition & 1 deletion tests/expand/unsafe_unpin/tuple_struct.expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const _: () = {
}
}
}
#[forbid(safe_packed_borrows)]
#[forbid(unaligned_references, safe_packed_borrows)]
fn __assert_not_repr_packed<T, U>(this: &TupleStruct<T, U>) {
let _ = &this.0;
let _ = &this.1;
Expand Down
Loading

0 comments on commit 6539cfd

Please sign in to comment.