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

Prepare for removal of safe_packed_borrows lint #321

Merged
merged 2 commits into from
Mar 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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