From 2c9556d28aef345102a394ea1256d7713748388a Mon Sep 17 00:00:00 2001 From: joboet Date: Tue, 25 Jun 2024 23:43:19 +0200 Subject: [PATCH 1/4] fix UI test, simplify error message --- library/core/src/fmt/mod.rs | 6 ++++++ tests/ui/fmt/send-sync.stderr | 30 ++++++------------------------ 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index c25bc5a1b13..3bcd4be1834 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -459,6 +459,12 @@ impl<'a> Arguments<'a> { } } +// Manually implementing these results in better error messages. +#[stable(feature = "rust1", since = "1.0.0")] +impl !Send for Arguments<'_> {} +#[stable(feature = "rust1", since = "1.0.0")] +impl !Sync for Arguments<'_> {} + #[stable(feature = "rust1", since = "1.0.0")] impl Debug for Arguments<'_> { fn fmt(&self, fmt: &mut Formatter<'_>) -> Result { diff --git a/tests/ui/fmt/send-sync.stderr b/tests/ui/fmt/send-sync.stderr index bebf575d9a7..dff9b23ba67 100644 --- a/tests/ui/fmt/send-sync.stderr +++ b/tests/ui/fmt/send-sync.stderr @@ -1,45 +1,27 @@ -error[E0277]: `core::fmt::rt::Opaque` cannot be shared between threads safely +error[E0277]: `Arguments<'_>` cannot be sent between threads safely --> $DIR/send-sync.rs:8:10 | LL | send(format_args!("{:?}", c)); - | ---- ^^^^^^^^^^^^^^^^^^^^^^^ `core::fmt::rt::Opaque` cannot be shared between threads safely + | ---- ^^^^^^^^^^^^^^^^^^^^^^^ `Arguments<'_>` cannot be sent between threads safely | | | required by a bound introduced by this call | - = help: within `[core::fmt::rt::Argument<'_>]`, the trait `Sync` is not implemented for `core::fmt::rt::Opaque`, which is required by `Arguments<'_>: Send` - = note: required because it appears within the type `&core::fmt::rt::Opaque` -note: required because it appears within the type `core::fmt::rt::ArgumentType<'_>` - --> $SRC_DIR/core/src/fmt/rt.rs:LL:COL -note: required because it appears within the type `core::fmt::rt::Argument<'_>` - --> $SRC_DIR/core/src/fmt/rt.rs:LL:COL - = note: required because it appears within the type `[core::fmt::rt::Argument<'_>]` - = note: required for `&[core::fmt::rt::Argument<'_>]` to implement `Send` -note: required because it appears within the type `Arguments<'_>` - --> $SRC_DIR/core/src/fmt/mod.rs:LL:COL + = help: the trait `Send` is not implemented for `Arguments<'_>` note: required by a bound in `send` --> $DIR/send-sync.rs:1:12 | LL | fn send(_: T) {} | ^^^^ required by this bound in `send` -error[E0277]: `core::fmt::rt::Opaque` cannot be shared between threads safely +error[E0277]: `Arguments<'_>` cannot be shared between threads safely --> $DIR/send-sync.rs:9:10 | LL | sync(format_args!("{:?}", c)); - | ---- ^^^^^^^^^^^^^^^^^^^^^^^ `core::fmt::rt::Opaque` cannot be shared between threads safely + | ---- ^^^^^^^^^^^^^^^^^^^^^^^ `Arguments<'_>` cannot be shared between threads safely | | | required by a bound introduced by this call | - = help: within `Arguments<'_>`, the trait `Sync` is not implemented for `core::fmt::rt::Opaque`, which is required by `Arguments<'_>: Sync` - = note: required because it appears within the type `&core::fmt::rt::Opaque` -note: required because it appears within the type `core::fmt::rt::ArgumentType<'_>` - --> $SRC_DIR/core/src/fmt/rt.rs:LL:COL -note: required because it appears within the type `core::fmt::rt::Argument<'_>` - --> $SRC_DIR/core/src/fmt/rt.rs:LL:COL - = note: required because it appears within the type `[core::fmt::rt::Argument<'_>]` - = note: required because it appears within the type `&[core::fmt::rt::Argument<'_>]` -note: required because it appears within the type `Arguments<'_>` - --> $SRC_DIR/core/src/fmt/mod.rs:LL:COL + = help: the trait `Sync` is not implemented for `Arguments<'_>` note: required by a bound in `sync` --> $DIR/send-sync.rs:2:12 | From 23d1cc4b84ad6d966f5a686ac6a93daf0239c455 Mon Sep 17 00:00:00 2001 From: joboet Date: Wed, 26 Jun 2024 00:06:16 +0200 Subject: [PATCH 2/4] core: avoid `extern` types in formatting infrastructure --- library/core/src/fmt/rt.rs | 46 +++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/library/core/src/fmt/rt.rs b/library/core/src/fmt/rt.rs index 92626feabf3..5836ce59ec1 100644 --- a/library/core/src/fmt/rt.rs +++ b/library/core/src/fmt/rt.rs @@ -5,6 +5,7 @@ use super::*; use crate::hint::unreachable_unchecked; +use crate::ptr::NonNull; #[lang = "format_placeholder"] #[derive(Copy, Clone)] @@ -66,7 +67,13 @@ pub(super) enum Flag { #[derive(Copy, Clone)] enum ArgumentType<'a> { - Placeholder { value: &'a Opaque, formatter: fn(&Opaque, &mut Formatter<'_>) -> Result }, + Placeholder { + // INVARIANT: if `formatter` had type `fn(&T, _) -> _` then `value` + // was derived from a `&T` with lifetime `'a`. + value: NonNull<()>, + formatter: unsafe fn(NonNull<()>, &mut Formatter<'_>) -> Result, + _lifetime: PhantomData<&'a ()>, + }, Count(usize), } @@ -90,21 +97,15 @@ pub struct Argument<'a> { impl<'a> Argument<'a> { #[inline(always)] fn new<'b, T>(x: &'b T, f: fn(&T, &mut Formatter<'_>) -> Result) -> Argument<'b> { - // SAFETY: `mem::transmute(x)` is safe because - // 1. `&'b T` keeps the lifetime it originated with `'b` - // (so as to not have an unbounded lifetime) - // 2. `&'b T` and `&'b Opaque` have the same memory layout - // (when `T` is `Sized`, as it is here) - // `mem::transmute(f)` is safe since `fn(&T, &mut Formatter<'_>) -> Result` - // and `fn(&Opaque, &mut Formatter<'_>) -> Result` have the same ABI - // (as long as `T` is `Sized`) - unsafe { - Argument { - ty: ArgumentType::Placeholder { - formatter: mem::transmute(f), - value: mem::transmute(x), - }, - } + Argument { + // INVARIANT: this creates an `ArgumentType<'b>` from a `&'b T` and + // a `fn(&T, ...)`, so the invariant is maintained. + ty: ArgumentType::Placeholder { + value: NonNull::from(x).cast(), + // SAFETY: function pointers always have the same layout. + formatter: unsafe { mem::transmute(f) }, + _lifetime: PhantomData, + }, } } @@ -162,7 +163,14 @@ impl<'a> Argument<'a> { #[inline(always)] pub(super) unsafe fn fmt(&self, f: &mut Formatter<'_>) -> Result { match self.ty { - ArgumentType::Placeholder { formatter, value } => formatter(value, f), + // SAFETY: + // Because of the invariant that if `formatter` had the type + // `fn(&T, _) -> _` then `value` has type `&'b T` where `'b` is + // the lifetime of the `ArgumentType`, and because references + // and `NonNull` are ABI-compatible, this is completely equivalent + // to calling the original function passed to `new` with the + // original reference, which is sound. + ArgumentType::Placeholder { formatter, value, .. } => unsafe { formatter(value, f) }, // SAFETY: the caller promised this. ArgumentType::Count(_) => unsafe { unreachable_unchecked() }, } @@ -208,7 +216,3 @@ impl UnsafeArg { Self { _private: () } } } - -extern "C" { - type Opaque; -} From 7526416ba6bcb3e0b51bfcdc93544a9552f9568e Mon Sep 17 00:00:00 2001 From: joboet Date: Wed, 26 Jun 2024 00:06:27 +0200 Subject: [PATCH 3/4] update coverage test --- tests/coverage/issue-83601.cov-map | 6 +++--- tests/coverage/issue-84561.cov-map | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/coverage/issue-83601.cov-map b/tests/coverage/issue-83601.cov-map index ddb4407881a..f2447e3c92c 100644 --- a/tests/coverage/issue-83601.cov-map +++ b/tests/coverage/issue-83601.cov-map @@ -1,12 +1,12 @@ Function name: issue_83601::main -Raw bytes (21): 0x[01, 01, 01, 05, 00, 03, 01, 06, 01, 02, 1c, 05, 03, 09, 01, 1c, 02, 02, 05, 03, 02] +Raw bytes (21): 0x[01, 01, 01, 05, 09, 03, 01, 06, 01, 02, 1c, 05, 03, 09, 01, 1c, 02, 02, 05, 03, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 1 -- expression 0 operands: lhs = Counter(1), rhs = Zero +- expression 0 operands: lhs = Counter(1), rhs = Counter(2) Number of file 0 mappings: 3 - Code(Counter(0)) at (prev + 6, 1) to (start + 2, 28) - Code(Counter(1)) at (prev + 3, 9) to (start + 1, 28) - Code(Expression(0, Sub)) at (prev + 2, 5) to (start + 3, 2) - = (c1 - Zero) + = (c1 - c2) diff --git a/tests/coverage/issue-84561.cov-map b/tests/coverage/issue-84561.cov-map index c4087d9369d..d3c4671e0ba 100644 --- a/tests/coverage/issue-84561.cov-map +++ b/tests/coverage/issue-84561.cov-map @@ -54,15 +54,15 @@ Number of file 0 mappings: 1 - Code(Counter(0)) at (prev + 167, 9) to (start + 2, 10) Function name: issue_84561::test3 -Raw bytes (375): 0x[01, 01, 31, 05, 00, 0d, 00, 15, 00, 12, 00, 15, 00, 21, 00, 1e, 00, 21, 00, 31, 00, 3d, 00, 2e, 45, 3d, 00, 42, 49, 45, 00, 3f, 51, 42, 49, 45, 00, 7a, 55, 51, 00, 7a, 55, 51, 00, 77, 5d, 7a, 55, 51, 00, 77, 61, 7a, 55, 51, 00, 72, 65, 77, 61, 7a, 55, 51, 00, 75, be, 01, c2, 01, 79, 69, 6d, 69, 6d, 69, 6d, c2, 01, 00, 69, 6d, c2, 01, 79, 69, 6d, bb, 01, 7d, 75, be, 01, c2, 01, 79, 69, 6d, b6, 01, 00, bb, 01, 7d, 75, be, 01, c2, 01, 79, 69, 6d, 33, 01, 08, 01, 03, 1c, 05, 04, 09, 01, 1c, 02, 02, 05, 04, 1f, 0d, 05, 05, 00, 1f, 06, 01, 05, 00, 1f, 15, 01, 09, 01, 1c, 12, 02, 05, 00, 1f, 0e, 01, 05, 00, 0f, 00, 00, 20, 00, 30, 21, 01, 05, 03, 0f, 00, 03, 20, 00, 30, 00, 00, 33, 00, 41, 00, 00, 4b, 00, 5a, 1e, 01, 05, 00, 0f, 00, 05, 09, 03, 10, 00, 05, 0d, 00, 1b, 00, 02, 0d, 00, 1c, 1a, 04, 09, 05, 06, 31, 06, 05, 03, 06, 22, 04, 05, 03, 06, 3d, 04, 09, 04, 06, 2e, 05, 08, 00, 0f, 45, 01, 09, 03, 0a, 2a, 05, 09, 03, 0a, 3f, 05, 08, 00, 0f, 51, 01, 09, 00, 13, 00, 03, 0d, 00, 1d, 3a, 03, 09, 00, 13, 00, 03, 0d, 00, 1d, 77, 03, 05, 00, 0f, 77, 01, 0c, 00, 13, 5d, 01, 0d, 00, 13, 56, 02, 0d, 00, 13, 72, 04, 05, 02, 13, 65, 03, 0d, 00, 13, 6e, 02, 0d, 00, 13, bb, 01, 03, 05, 00, 0f, 69, 01, 0c, 00, 13, 6d, 01, 0d, 03, 0e, 75, 04, 0d, 00, 13, c2, 01, 02, 0d, 00, 17, c2, 01, 01, 14, 00, 1b, 00, 01, 15, 00, 1b, 92, 01, 02, 15, 00, 1b, be, 01, 04, 0d, 00, 13, 7d, 03, 09, 00, 19, b6, 01, 02, 05, 00, 0f, b2, 01, 03, 09, 00, 22, 00, 02, 05, 00, 0f, 00, 03, 09, 00, 2c, 00, 02, 01, 00, 02] +Raw bytes (375): 0x[01, 01, 31, 05, 09, 0d, 00, 15, 19, 12, 00, 15, 19, 21, 00, 1e, 00, 21, 00, 31, 00, 3d, 00, 2e, 45, 3d, 00, 42, 49, 45, 00, 3f, 51, 42, 49, 45, 00, 7a, 55, 51, 00, 7a, 55, 51, 00, 77, 5d, 7a, 55, 51, 00, 77, 61, 7a, 55, 51, 00, 72, 65, 77, 61, 7a, 55, 51, 00, 75, be, 01, c2, 01, 79, 69, 6d, 69, 6d, 69, 6d, c2, 01, 00, 69, 6d, c2, 01, 79, 69, 6d, bb, 01, 7d, 75, be, 01, c2, 01, 79, 69, 6d, b6, 01, 00, bb, 01, 7d, 75, be, 01, c2, 01, 79, 69, 6d, 33, 01, 08, 01, 03, 1c, 05, 04, 09, 01, 1c, 02, 02, 05, 04, 1f, 0d, 05, 05, 00, 1f, 06, 01, 05, 00, 1f, 15, 01, 09, 01, 1c, 12, 02, 05, 00, 1f, 0e, 01, 05, 00, 0f, 00, 00, 20, 00, 30, 21, 01, 05, 03, 0f, 00, 03, 20, 00, 30, 00, 00, 33, 00, 41, 00, 00, 4b, 00, 5a, 1e, 01, 05, 00, 0f, 00, 05, 09, 03, 10, 00, 05, 0d, 00, 1b, 00, 02, 0d, 00, 1c, 1a, 04, 09, 05, 06, 31, 06, 05, 03, 06, 22, 04, 05, 03, 06, 3d, 04, 09, 04, 06, 2e, 05, 08, 00, 0f, 45, 01, 09, 03, 0a, 2a, 05, 09, 03, 0a, 3f, 05, 08, 00, 0f, 51, 01, 09, 00, 13, 00, 03, 0d, 00, 1d, 3a, 03, 09, 00, 13, 00, 03, 0d, 00, 1d, 77, 03, 05, 00, 0f, 77, 01, 0c, 00, 13, 5d, 01, 0d, 00, 13, 56, 02, 0d, 00, 13, 72, 04, 05, 02, 13, 65, 03, 0d, 00, 13, 6e, 02, 0d, 00, 13, bb, 01, 03, 05, 00, 0f, 69, 01, 0c, 00, 13, 6d, 01, 0d, 03, 0e, 75, 04, 0d, 00, 13, c2, 01, 02, 0d, 00, 17, c2, 01, 01, 14, 00, 1b, 00, 01, 15, 00, 1b, 92, 01, 02, 15, 00, 1b, be, 01, 04, 0d, 00, 13, 7d, 03, 09, 00, 19, b6, 01, 02, 05, 00, 0f, b2, 01, 03, 09, 00, 22, 00, 02, 05, 00, 0f, 00, 03, 09, 00, 2c, 00, 02, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 49 -- expression 0 operands: lhs = Counter(1), rhs = Zero +- expression 0 operands: lhs = Counter(1), rhs = Counter(2) - expression 1 operands: lhs = Counter(3), rhs = Zero -- expression 2 operands: lhs = Counter(5), rhs = Zero +- expression 2 operands: lhs = Counter(5), rhs = Counter(6) - expression 3 operands: lhs = Expression(4, Sub), rhs = Zero -- expression 4 operands: lhs = Counter(5), rhs = Zero +- expression 4 operands: lhs = Counter(5), rhs = Counter(6) - expression 5 operands: lhs = Counter(8), rhs = Zero - expression 6 operands: lhs = Expression(7, Sub), rhs = Zero - expression 7 operands: lhs = Counter(8), rhs = Zero @@ -111,15 +111,15 @@ Number of file 0 mappings: 51 - Code(Counter(0)) at (prev + 8, 1) to (start + 3, 28) - Code(Counter(1)) at (prev + 4, 9) to (start + 1, 28) - Code(Expression(0, Sub)) at (prev + 2, 5) to (start + 4, 31) - = (c1 - Zero) + = (c1 - c2) - Code(Counter(3)) at (prev + 5, 5) to (start + 0, 31) - Code(Expression(1, Sub)) at (prev + 1, 5) to (start + 0, 31) = (c3 - Zero) - Code(Counter(5)) at (prev + 1, 9) to (start + 1, 28) - Code(Expression(4, Sub)) at (prev + 2, 5) to (start + 0, 31) - = (c5 - Zero) + = (c5 - c6) - Code(Expression(3, Sub)) at (prev + 1, 5) to (start + 0, 15) - = ((c5 - Zero) - Zero) + = ((c5 - c6) - Zero) - Code(Zero) at (prev + 0, 32) to (start + 0, 48) - Code(Counter(8)) at (prev + 1, 5) to (start + 3, 15) - Code(Zero) at (prev + 3, 32) to (start + 0, 48) From 7e7d0a959d9a0450ac3596e842dc49e8e6618bc6 Mon Sep 17 00:00:00 2001 From: joboet Date: Thu, 27 Jun 2024 12:16:46 +0200 Subject: [PATCH 4/4] core: improve comment Co-authored-by: Ralf Jung --- library/core/src/fmt/rt.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/core/src/fmt/rt.rs b/library/core/src/fmt/rt.rs index 5836ce59ec1..65a4d537cc7 100644 --- a/library/core/src/fmt/rt.rs +++ b/library/core/src/fmt/rt.rs @@ -68,8 +68,8 @@ pub(super) enum Flag { #[derive(Copy, Clone)] enum ArgumentType<'a> { Placeholder { - // INVARIANT: if `formatter` had type `fn(&T, _) -> _` then `value` - // was derived from a `&T` with lifetime `'a`. + // INVARIANT: `formatter` has type `fn(&T, _) -> _` for some `T`, and `value` + // was derived from a `&'a T`. value: NonNull<()>, formatter: unsafe fn(NonNull<()>, &mut Formatter<'_>) -> Result, _lifetime: PhantomData<&'a ()>,