From 43a865ad540a3d8218eeb62ba4e7f9987cfa1c0e Mon Sep 17 00:00:00 2001 From: joboet Date: Tue, 25 Jun 2024 23:43:19 +0200 Subject: [PATCH 1/3] fix UI test, simplify error message --- core/src/fmt/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/src/fmt/mod.rs b/core/src/fmt/mod.rs index c25bc5a1b13c9..3bcd4be183465 100644 --- a/core/src/fmt/mod.rs +++ b/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 { From f6fdef3996acb8207d61b222d8205a6ad5da7526 Mon Sep 17 00:00:00 2001 From: joboet Date: Wed, 26 Jun 2024 00:06:16 +0200 Subject: [PATCH 2/3] core: avoid `extern` types in formatting infrastructure --- core/src/fmt/rt.rs | 46 +++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/core/src/fmt/rt.rs b/core/src/fmt/rt.rs index 92626feabf3d7..5836ce59ec1dc 100644 --- a/core/src/fmt/rt.rs +++ b/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 8db57c2b618bde408e656c69b963ef9641880e8a Mon Sep 17 00:00:00 2001 From: joboet Date: Thu, 27 Jun 2024 12:16:46 +0200 Subject: [PATCH 3/3] core: improve comment Co-authored-by: Ralf Jung --- core/src/fmt/rt.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/fmt/rt.rs b/core/src/fmt/rt.rs index 5836ce59ec1dc..65a4d537cc74d 100644 --- a/core/src/fmt/rt.rs +++ b/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 ()>,