From c75e8d46c2fc576661c01f9eadb0866b3367ca4b Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Wed, 3 Dec 2014 22:46:09 +0200 Subject: [PATCH 1/3] core: remove the dead function fmt::argumentstr. --- src/libcore/fmt/mod.rs | 8 -------- src/libstd/fmt.rs | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/libcore/fmt/mod.rs b/src/libcore/fmt/mod.rs index 9b67cdd3e1275..6180daebcc2d4 100644 --- a/src/libcore/fmt/mod.rs +++ b/src/libcore/fmt/mod.rs @@ -543,14 +543,6 @@ pub fn argument<'a, T>(f: extern "Rust" fn(&T, &mut Formatter) -> Result, } } -/// When the compiler determines that the type of an argument *must* be a string -/// (such as for select), then it invokes this method. -#[doc(hidden)] #[inline] -#[experimental = "implementation detail of the `format_args!` macro"] -pub fn argumentstr<'a>(s: &'a &str) -> Argument<'a> { - argument(Show::fmt, s) -} - /// When the compiler determines that the type of an argument *must* be a uint /// (such as for plural), then it invokes this method. #[doc(hidden)] #[inline] diff --git a/src/libstd/fmt.rs b/src/libstd/fmt.rs index 6a2047d1cef30..d0c9df9d914e7 100644 --- a/src/libstd/fmt.rs +++ b/src/libstd/fmt.rs @@ -418,7 +418,7 @@ pub use core::fmt::Error; pub use core::fmt::{Argument, Arguments, write, radix, Radix, RadixFmt}; #[doc(hidden)] -pub use core::fmt::{argument, argumentstr, argumentuint}; +pub use core::fmt::{argument, argumentuint}; /// The format function takes a precompiled format string and a list of /// arguments, to return the resulting formatted string. From fe4fdcc0f67272de0c9f40da8b699a48bffb719f Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Wed, 3 Dec 2014 22:56:39 +0200 Subject: [PATCH 2/3] core: make the public fmt API completely safe. --- src/libcore/fmt/mod.rs | 81 ++++++++++++++++++++++++------------- src/libsyntax/ext/format.rs | 33 ++++----------- 2 files changed, 61 insertions(+), 53 deletions(-) diff --git a/src/libcore/fmt/mod.rs b/src/libcore/fmt/mod.rs index 6180daebcc2d4..8b2ffd90ef715 100644 --- a/src/libcore/fmt/mod.rs +++ b/src/libcore/fmt/mod.rs @@ -92,6 +92,9 @@ pub struct Formatter<'a> { args: &'a [Argument<'a>], } +// NB. Argument is essentially an optimized partially applied formatting function, +// equivalent to `exists T.(&T, fn(&T, &mut Formatter) -> Result`. + enum Void {} /// This struct represents the generic "argument" which is taken by the Xprintf @@ -100,21 +103,47 @@ enum Void {} /// types, and then this struct is used to canonicalize arguments to one type. #[experimental = "implementation detail of the `format_args!` macro"] pub struct Argument<'a> { - formatter: extern "Rust" fn(&Void, &mut Formatter) -> Result, value: &'a Void, + formatter: fn(&Void, &mut Formatter) -> Result, +} + +impl<'a> Argument<'a> { + #[inline(never)] + fn show_uint(x: &uint, f: &mut Formatter) -> Result { + Show::fmt(x, f) + } + + fn new<'a, T>(x: &'a T, f: fn(&T, &mut Formatter) -> Result) -> Argument<'a> { + unsafe { + Argument { + formatter: mem::transmute(f), + value: mem::transmute(x) + } + } + } + + fn from_uint<'a>(x: &'a uint) -> Argument<'a> { + Argument::new(x, Argument::show_uint) + } + + fn as_uint(&self) -> Option { + if self.formatter as uint == Argument::show_uint as uint { + Some(unsafe { *(self.value as *const _ as *const uint) }) + } else { + None + } + } } impl<'a> Arguments<'a> { /// When using the format_args!() macro, this function is used to generate the - /// Arguments structure. The compiler inserts an `unsafe` block to call this, - /// which is valid because the compiler performs all necessary validation to - /// ensure that the resulting call to format/write would be safe. + /// Arguments structure. #[doc(hidden)] #[inline] #[experimental = "implementation detail of the `format_args!` macro"] - pub unsafe fn new<'a>(pieces: &'static [&'static str], - args: &'a [Argument<'a>]) -> Arguments<'a> { + pub fn new<'a>(pieces: &'a [&'a str], + args: &'a [Argument<'a>]) -> Arguments<'a> { Arguments { - pieces: mem::transmute(pieces), + pieces: pieces, fmt: None, args: args } @@ -122,15 +151,18 @@ impl<'a> Arguments<'a> { /// This function is used to specify nonstandard formatting parameters. /// The `pieces` array must be at least as long as `fmt` to construct - /// a valid Arguments structure. + /// a valid Arguments structure. Also, any `Count` within `fmt` that is + /// `CountIsParam` or `CountIsNextParam` has to point to an argument + /// created with `argumentuint`. However, failing to do so doesn't cause + /// unsafety, but will ignore invalid . #[doc(hidden)] #[inline] #[experimental = "implementation detail of the `format_args!` macro"] - pub unsafe fn with_placeholders<'a>(pieces: &'static [&'static str], - fmt: &'static [rt::Argument<'static>], - args: &'a [Argument<'a>]) -> Arguments<'a> { + pub fn with_placeholders<'a>(pieces: &'a [&'a str], + fmt: &'a [rt::Argument<'a>], + args: &'a [Argument<'a>]) -> Arguments<'a> { Arguments { - pieces: mem::transmute(pieces), - fmt: Some(mem::transmute(fmt)), + pieces: pieces, + fmt: Some(fmt), args: args } } @@ -312,15 +344,13 @@ impl<'a> Formatter<'a> { fn getcount(&mut self, cnt: &rt::Count) -> Option { match *cnt { - rt::CountIs(n) => { Some(n) } - rt::CountImplied => { None } + rt::CountIs(n) => Some(n), + rt::CountImplied => None, rt::CountIsParam(i) => { - let v = self.args[i].value; - unsafe { Some(*(v as *const _ as *const uint)) } + self.args[i].as_uint() } rt::CountIsNextParam => { - let v = self.curarg.next().unwrap().value; - unsafe { Some(*(v as *const _ as *const uint)) } + self.curarg.next().and_then(|arg| arg.as_uint()) } } } @@ -533,22 +563,17 @@ impl Show for Error { /// create the Argument structures that are passed into the `format` function. #[doc(hidden)] #[inline] #[experimental = "implementation detail of the `format_args!` macro"] -pub fn argument<'a, T>(f: extern "Rust" fn(&T, &mut Formatter) -> Result, +pub fn argument<'a, T>(f: fn(&T, &mut Formatter) -> Result, t: &'a T) -> Argument<'a> { - unsafe { - Argument { - formatter: mem::transmute(f), - value: mem::transmute(t) - } - } + Argument::new(t, f) } /// When the compiler determines that the type of an argument *must* be a uint -/// (such as for plural), then it invokes this method. +/// (such as for width and precision), then it invokes this method. #[doc(hidden)] #[inline] #[experimental = "implementation detail of the `format_args!` macro"] pub fn argumentuint<'a>(s: &'a uint) -> Argument<'a> { - argument(Show::fmt, s) + Argument::from_uint(s) } // Implementations of the core formatting traits diff --git a/src/libsyntax/ext/format.rs b/src/libsyntax/ext/format.rs index d7d6c636849a6..c8fed3dcd16f6 100644 --- a/src/libsyntax/ext/format.rs +++ b/src/libsyntax/ext/format.rs @@ -577,17 +577,11 @@ impl<'a, 'b> Context<'a, 'b> { } // Now create a vector containing all the arguments - let slicename = self.ecx.ident_of("__args_vec"); - { - let args = names.into_iter().map(|a| a.unwrap()); - let args = locals.into_iter().chain(args); - let args = self.ecx.expr_vec_slice(self.fmtsp, args.collect()); - lets.push(self.ecx.stmt_let(self.fmtsp, false, slicename, args)); - } + let args = locals.into_iter().chain(names.into_iter().map(|a| a.unwrap())); // Now create the fmt::Arguments struct with all our locals we created. let pieces = self.ecx.expr_ident(self.fmtsp, static_str_name); - let args_slice = self.ecx.expr_ident(self.fmtsp, slicename); + let args_slice = self.ecx.expr_vec_slice(self.fmtsp, args.collect()); let (fn_name, fn_args) = if self.all_pieces_simple { ("new", vec![pieces, args_slice]) @@ -602,29 +596,18 @@ impl<'a, 'b> Context<'a, 'b> { self.ecx.ident_of("Arguments"), self.ecx.ident_of(fn_name)), fn_args); - // We did all the work of making sure that the arguments - // structure is safe, so we can safely have an unsafe block. - let result = self.ecx.expr_block(P(ast::Block { - view_items: Vec::new(), - stmts: Vec::new(), - expr: Some(result), - id: ast::DUMMY_NODE_ID, - rules: ast::UnsafeBlock(ast::CompilerGenerated), - span: self.fmtsp, - })); - let resname = self.ecx.ident_of("__args"); - lets.push(self.ecx.stmt_let(self.fmtsp, false, resname, result)); - let res = self.ecx.expr_ident(self.fmtsp, resname); let result = match invocation { Call(e) => { let span = e.span; - self.ecx.expr_call(span, e, - vec!(self.ecx.expr_addr_of(span, res))) + self.ecx.expr_call(span, e, vec![ + self.ecx.expr_addr_of(span, result) + ]) } MethodCall(e, m) => { let span = e.span; - self.ecx.expr_method_call(span, e, m, - vec!(self.ecx.expr_addr_of(span, res))) + self.ecx.expr_method_call(span, e, m, vec![ + self.ecx.expr_addr_of(span, result) + ]) } }; let body = self.ecx.expr_block(self.ecx.block(self.fmtsp, lets, From 15ca63081b384bfcf43d5a065298bb6fbf2cbfe0 Mon Sep 17 00:00:00 2001 From: Eduard Burtescu Date: Mon, 8 Dec 2014 09:14:00 +0200 Subject: [PATCH 3/3] test: adjust pretty/issue-4264 for formatting changes. --- src/test/pretty/issue-4264.pp | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/test/pretty/issue-4264.pp b/src/test/pretty/issue-4264.pp index 94a168a74ebb5..8a2c2d35ef5ba 100644 --- a/src/test/pretty/issue-4264.pp +++ b/src/test/pretty/issue-4264.pp @@ -41,20 +41,6 @@ static __STATIC_FMTSTR: &'static [&'static str] = (&([("test" as &'static str)] as [&'static str, ..1]) as &'static [&'static str, ..1]); - let __args_vec = - (&([] as [core::fmt::Argument<'_>, ..0]) as - &[core::fmt::Argument<'_>, ..0]); - let __args = - (unsafe { - ((::std::fmt::Arguments::new as - unsafe fn(&'static [&'static str], &'a [core::fmt::Argument<'a>]) -> core::fmt::Arguments<'a>)((__STATIC_FMTSTR - as - &'static [&'static str]), - (__args_vec - as - &[core::fmt::Argument<'_>, ..0])) - as core::fmt::Arguments<'_>) - } as core::fmt::Arguments<'_>); @@ -64,7 +50,16 @@ ((::std::fmt::format as - fn(&core::fmt::Arguments<'_>) -> collections::string::String)((&(__args + fn(&core::fmt::Arguments<'_>) -> collections::string::String)((&((::std::fmt::Arguments::new + as + fn(&'a [&'a str], &'a [core::fmt::Argument<'a>]) -> core::fmt::Arguments<'a>)((__STATIC_FMTSTR + as + &'static [&'static str]), + (&([] + as + [core::fmt::Argument<'_>, ..0]) + as + &[core::fmt::Argument<'_>, ..0])) as core::fmt::Arguments<'_>) as