From cfe3db810b7991ef1144afaed4156ddc2586efef Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 12 Sep 2013 19:36:58 -0700 Subject: [PATCH 1/3] Reduce the amount of complexity in format! This renames the syntax-extension file to format from ifmt, and it also reduces the amount of complexity inside by defining all other macros in terms of format_args! --- src/libstd/fmt/mod.rs | 7 ++ src/libsyntax/ext/base.rs | 8 +- src/libsyntax/ext/expand.rs | 38 ++++--- src/libsyntax/ext/{ifmt.rs => format.rs} | 136 ++++++----------------- src/libsyntax/syntax.rs | 2 +- src/test/compile-fail/ifmt-bad-arg.rs | 8 +- 6 files changed, 74 insertions(+), 125 deletions(-) rename src/libsyntax/ext/{ifmt.rs => format.rs} (87%) diff --git a/src/libstd/fmt/mod.rs b/src/libstd/fmt/mod.rs index c7ab508ea6ec6..61024ee834b41 100644 --- a/src/libstd/fmt/mod.rs +++ b/src/libstd/fmt/mod.rs @@ -452,6 +452,13 @@ pub fn write(output: &mut io::Writer, args: &Arguments) { unsafe { write_unsafe(output, args.fmt, args.args) } } +/// The `writeln` function takes the same arguments as `write`, except that it +/// will also write a newline (`\n`) character at the end of the format string. +pub fn writeln(output: &mut io::Writer, args: &Arguments) { + unsafe { write_unsafe(output, args.fmt, args.args) } + output.write(['\n' as u8]); +} + /// The `write_unsafe` function takes an output stream, a precompiled format /// string, and a list of arguments. The arguments will be formatted according /// to the specified format string into the output stream provided. diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index 284801ab123c9..2bcfafc3bb483 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -155,14 +155,8 @@ pub fn syntax_expander_table() -> SyntaxEnv { @SE(IdentTT(ext::tt::macro_rules::add_new_extension, None))); syntax_expanders.insert(intern(&"fmt"), builtin_normal_tt_no_ctxt(ext::fmt::expand_syntax_ext)); - syntax_expanders.insert(intern(&"format"), - builtin_normal_tt_no_ctxt(ext::ifmt::expand_format)); - syntax_expanders.insert(intern(&"write"), - builtin_normal_tt_no_ctxt(ext::ifmt::expand_write)); - syntax_expanders.insert(intern(&"writeln"), - builtin_normal_tt_no_ctxt(ext::ifmt::expand_writeln)); syntax_expanders.insert(intern(&"format_args"), - builtin_normal_tt_no_ctxt(ext::ifmt::expand_format_args)); + builtin_normal_tt_no_ctxt(ext::format::expand_args)); syntax_expanders.insert( intern(&"auto_encode"), @SE(ItemDecorator(ext::auto_encode::expand_auto_encode))); diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 0c8986fc6d778..e4c69c1973aaa 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -715,11 +715,11 @@ pub fn std_macros() -> @str { } }) ) - macro_rules! error( ($($arg:tt)+) => (log!(1u32, $($arg)+)) ) - macro_rules! warn ( ($($arg:tt)+) => (log!(2u32, $($arg)+)) ) - macro_rules! info ( ($($arg:tt)+) => (log!(3u32, $($arg)+)) ) - macro_rules! debug( ($($arg:tt)+) => ( - if cfg!(debug) { log!(4u32, $($arg)+) } + macro_rules! error( ($($arg:tt)*) => (log!(1u32, $($arg)*)) ) + macro_rules! warn ( ($($arg:tt)*) => (log!(2u32, $($arg)*)) ) + macro_rules! info ( ($($arg:tt)*) => (log!(3u32, $($arg)*)) ) + macro_rules! debug( ($($arg:tt)*) => ( + if cfg!(debug) { log!(4u32, $($arg)*) } )) macro_rules! log2( @@ -730,11 +730,11 @@ pub fn std_macros() -> @str { } }) ) - macro_rules! error2( ($($arg:tt)+) => (log2!(1u32, $($arg)+)) ) - macro_rules! warn2 ( ($($arg:tt)+) => (log2!(2u32, $($arg)+)) ) - macro_rules! info2 ( ($($arg:tt)+) => (log2!(3u32, $($arg)+)) ) - macro_rules! debug2( ($($arg:tt)+) => ( - if cfg!(debug) { log2!(4u32, $($arg)+) } + macro_rules! error2( ($($arg:tt)*) => (log2!(1u32, $($arg)*)) ) + macro_rules! warn2 ( ($($arg:tt)*) => (log2!(2u32, $($arg)*)) ) + macro_rules! info2 ( ($($arg:tt)*) => (log2!(3u32, $($arg)*)) ) + macro_rules! debug2( ($($arg:tt)*) => ( + if cfg!(debug) { log2!(4u32, $($arg)*) } )) macro_rules! fail( @@ -753,8 +753,8 @@ pub fn std_macros() -> @str { () => ( fail!(\"explicit failure\") ); - ($($arg:tt)+) => ( - ::std::sys::FailWithCause::fail_with(format!($($arg)+), file!(), line!()) + ($($arg:tt)*) => ( + ::std::sys::FailWithCause::fail_with(format!($($arg)*), file!(), line!()) ) ) @@ -958,17 +958,25 @@ pub fn std_macros() -> @str { ) ) + macro_rules! format(($($arg:tt)*) => ( + format_args!(::std::fmt::format, $($arg)*) + )) + macro_rules! write(($dst:expr, $($arg:tt)*) => ( + format_args!(|args| { ::std::fmt::write($dst, args) }, $($arg)*) + )) + macro_rules! writeln(($dst:expr, $($arg:tt)*) => ( + format_args!(|args| { ::std::fmt::writeln($dst, args) }, $($arg)*) + )) // FIXME(#6846) once stdio is redesigned, this shouldn't perform an // allocation but should rather delegate to an invocation of // write! instead of format! macro_rules! print ( - ($($arg:tt)+) => (::std::io::print(format!($($arg)+))) + ($($arg:tt)*) => (::std::io::print(format!($($arg)*))) ) - // FIXME(#6846) once stdio is redesigned, this shouldn't perform an // allocation but should rather delegate to an io::Writer macro_rules! println ( - ($($arg:tt)+) => (::std::io::println(format!($($arg)+))) + ($($arg:tt)*) => (::std::io::println(format!($($arg)*))) ) // NOTE: use this after a snapshot lands to abstract the details diff --git a/src/libsyntax/ext/ifmt.rs b/src/libsyntax/ext/format.rs similarity index 87% rename from src/libsyntax/ext/ifmt.rs rename to src/libsyntax/ext/format.rs index 2f040ef251923..9f4e55b1a92ce 100644 --- a/src/libsyntax/ext/ifmt.rs +++ b/src/libsyntax/ext/format.rs @@ -54,21 +54,16 @@ impl Context { /// Parses the arguments from the given list of tokens, returning None if /// there's a parse error so we can continue parsing other fmt! expressions. fn parse_args(&mut self, sp: Span, - leading_expr: bool, - tts: &[ast::token_tree]) -> (Option<@ast::Expr>, - Option<@ast::Expr>) { + tts: &[ast::token_tree]) -> (@ast::Expr, Option<@ast::Expr>) { let p = rsparse::new_parser_from_tts(self.ecx.parse_sess(), self.ecx.cfg(), tts.to_owned()); - // If we want a leading expression, parse it here - let extra = if leading_expr { - let e = Some(p.parse_expr()); - if !p.eat(&token::COMMA) { - self.ecx.span_err(sp, "expected token: `,`"); - return (e, None); - } - e - } else { None }; + // Parse the leading function expression (maybe a block, maybe a path) + let extra = p.parse_expr(); + if !p.eat(&token::COMMA) { + self.ecx.span_err(sp, "expected token: `,`"); + return (extra, None); + } if *p.token == token::EOF { self.ecx.span_err(sp, "requires at least a format string argument"); @@ -547,7 +542,7 @@ impl Context { /// Actually builds the expression which the ifmt! block will be expanded /// to - fn to_expr(&self, extra: Option<@ast::Expr>, f: Option<&str>) -> @ast::Expr { + fn to_expr(&self, extra: @ast::Expr) -> @ast::Expr { let mut lets = ~[]; let mut locals = ~[]; let mut names = vec::from_fn(self.name_positions.len(), |_| None); @@ -614,68 +609,33 @@ impl Context { self.ecx.expr_ident(e.span, lname))); } + // Now create the fmt::Arguments struct with all our locals we created. let args = names.move_iter().map(|a| a.unwrap()); let mut args = locals.move_iter().chain(args); - - let result = match f { - // Invocation of write!()/format!(), call the function and we're - // done. - Some(f) => { - let mut fmt_args = match extra { - Some(e) => ~[e], None => ~[] - }; - fmt_args.push(self.ecx.expr_ident(self.fmtsp, static_name)); - fmt_args.push(self.ecx.expr_vec_slice(self.fmtsp, - args.collect())); - - let result = self.ecx.expr_call_global(self.fmtsp, ~[ - self.ecx.ident_of("std"), - self.ecx.ident_of("fmt"), - self.ecx.ident_of(f), - ], fmt_args); - - // sprintf is unsafe, but we just went through a lot of work to - // validate that our call is save, so inject the unsafe block - // for the user. - self.ecx.expr_block(ast::Block { - view_items: ~[], - stmts: ~[], - expr: Some(result), - id: ast::DUMMY_NODE_ID, - rules: ast::UnsafeBlock(ast::CompilerGenerated), - span: self.fmtsp, - }) - } - - // Invocation of format_args!() - None => { - let fmt = self.ecx.expr_ident(self.fmtsp, static_name); - let args = self.ecx.expr_vec_slice(self.fmtsp, args.collect()); - let result = self.ecx.expr_call_global(self.fmtsp, ~[ - self.ecx.ident_of("std"), - self.ecx.ident_of("fmt"), - self.ecx.ident_of("Arguments"), - self.ecx.ident_of("new"), - ], ~[fmt, 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(ast::Block { - view_items: ~[], - stmts: ~[], - expr: Some(result), - id: ast::DUMMY_NODE_ID, - rules: ast::UnsafeBlock(ast::CompilerGenerated), - span: self.fmtsp, - }); - let extra = extra.unwrap(); - 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); - self.ecx.expr_call(extra.span, extra, ~[ - self.ecx.expr_addr_of(extra.span, res)]) - } - }; + let fmt = self.ecx.expr_ident(self.fmtsp, static_name); + let args = self.ecx.expr_vec_slice(self.fmtsp, args.collect()); + let result = self.ecx.expr_call_global(self.fmtsp, ~[ + self.ecx.ident_of("std"), + self.ecx.ident_of("fmt"), + self.ecx.ident_of("Arguments"), + self.ecx.ident_of("new"), + ], ~[fmt, 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(ast::Block { + view_items: ~[], + stmts: ~[], + 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 = self.ecx.expr_call(extra.span, extra, ~[ + self.ecx.expr_addr_of(extra.span, res)]); self.ecx.expr_block(self.ecx.block(self.fmtsp, lets, Some(result))) } @@ -740,29 +700,8 @@ impl Context { } } -pub fn expand_format(ecx: @ExtCtxt, sp: Span, - tts: &[ast::token_tree]) -> base::MacResult { - expand_ifmt(ecx, sp, tts, false, false, Some("format_unsafe")) -} - -pub fn expand_write(ecx: @ExtCtxt, sp: Span, - tts: &[ast::token_tree]) -> base::MacResult { - expand_ifmt(ecx, sp, tts, true, false, Some("write_unsafe")) -} - -pub fn expand_writeln(ecx: @ExtCtxt, sp: Span, - tts: &[ast::token_tree]) -> base::MacResult { - expand_ifmt(ecx, sp, tts, true, true, Some("write_unsafe")) -} - -pub fn expand_format_args(ecx: @ExtCtxt, sp: Span, - tts: &[ast::token_tree]) -> base::MacResult { - expand_ifmt(ecx, sp, tts, true, false, None) -} - -fn expand_ifmt(ecx: @ExtCtxt, sp: Span, tts: &[ast::token_tree], - leading_arg: bool, append_newline: bool, - function: Option<&str>) -> base::MacResult { +pub fn expand_args(ecx: @ExtCtxt, sp: Span, + tts: &[ast::token_tree]) -> base::MacResult { let mut cx = Context { ecx: ecx, args: ~[], @@ -776,14 +715,13 @@ fn expand_ifmt(ecx: @ExtCtxt, sp: Span, tts: &[ast::token_tree], method_statics: ~[], fmtsp: sp, }; - let (extra, efmt) = match cx.parse_args(sp, leading_arg, tts) { + let (extra, efmt) = match cx.parse_args(sp, tts) { (extra, Some(e)) => (extra, e), (_, None) => { return MRExpr(ecx.expr_uint(sp, 2)); } }; cx.fmtsp = efmt.span; let fmt = expr_to_str(ecx, efmt, "format argument must be a string literal."); - let fmt = if append_newline { fmt + "\n" } else { fmt.to_owned() }; let mut err = false; do parse::parse_error::cond.trap(|m| { @@ -814,5 +752,5 @@ fn expand_ifmt(ecx: @ExtCtxt, sp: Span, tts: &[ast::token_tree], } } - MRExpr(cx.to_expr(extra, function)) + MRExpr(cx.to_expr(extra)) } diff --git a/src/libsyntax/syntax.rs b/src/libsyntax/syntax.rs index fec3b88aab411..48270702e0dd5 100644 --- a/src/libsyntax/syntax.rs +++ b/src/libsyntax/syntax.rs @@ -72,7 +72,7 @@ pub mod ext { pub mod cfg; pub mod fmt; - pub mod ifmt; + pub mod format; pub mod env; pub mod bytes; pub mod concat_idents; diff --git a/src/test/compile-fail/ifmt-bad-arg.rs b/src/test/compile-fail/ifmt-bad-arg.rs index 5da73a495f6ed..35085feaf1505 100644 --- a/src/test/compile-fail/ifmt-bad-arg.rs +++ b/src/test/compile-fail/ifmt-bad-arg.rs @@ -11,7 +11,6 @@ fn main() { // bad arguments to the format! call - format!(); //~ ERROR: requires at least a format string format!("{}"); //~ ERROR: invalid reference to argument format!("{1}", 1); //~ ERROR: invalid reference to argument `1` @@ -30,8 +29,6 @@ fn main() { format!("{foo}", foo=1, foo=2); //~ ERROR: duplicate argument format!("#"); //~ ERROR: `#` reference used format!("", foo=1, 2); //~ ERROR: positional arguments cannot follow - format!("" 1); //~ ERROR: expected token: `,` - format!("", 1 1); //~ ERROR: expected token: `,` format!("{0, select, a{} a{} other{}}", "a"); //~ ERROR: duplicate selector format!("{0, plural, =1{} =1{} other{}}", 1u); //~ ERROR: duplicate selector @@ -74,4 +71,9 @@ fn main() { format!("foo } bar"); //~ ERROR: unmatched `}` found format!("foo }"); //~ ERROR: unmatched `}` found + + // FIXME(#5794) the spans on these errors are pretty terrible + //format!(); + //format!("" 1); + //format!("", 1 1); } From fd8c05ccc99642ed6be3516f24c8d876f2046eb4 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 12 Sep 2013 20:08:50 -0700 Subject: [PATCH 2/3] Document all of the format! related macros --- src/libstd/fmt/mod.rs | 76 ++++++++++++++++++++++++++- src/test/compile-fail/ifmt-bad-arg.rs | 7 ++- 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/src/libstd/fmt/mod.rs b/src/libstd/fmt/mod.rs index 61024ee834b41..cad9f14bda734 100644 --- a/src/libstd/fmt/mod.rs +++ b/src/libstd/fmt/mod.rs @@ -133,7 +133,7 @@ is `?` which is defined for all types by default. When implementing a format trait for your own time, you will have to implement a method of the signature: -~~~ +~~~{.rust} fn fmt(value: &T, f: &mut std::fmt::Formatter); ~~~ @@ -144,6 +144,78 @@ values of these parameters will be listed in the fields of the `Formatter` struct. In order to help with this, the `Formatter` struct also provides some helper methods. +### Related macros + +There are a number of related macros in the `format!` family. The ones that are +currently implemented are: + +~~~{.rust} +format! // described above +write! // first argument is a &mut rt::io::Writer, the destination +writeln! // same as write but appends a newline +print! // the format string is printed to the standard output +println! // same as print but appends a newline +format_args! // described below. +~~~ + + +#### `write!` + +This and `writeln` are two macros which are used to emit the format string to a +specified stream. This is used to prevent intermediate allocations of format +strings and instead directly write the output. Under the hood, this function is +actually invoking the `write` function defined in this module. Example usage is: + +~~~{.rust} +use std::rt::io; + +let mut w = io::mem::MemWriter::new(); +write!(&mut w as &mut io::Writer, "Hello {}!", "world"); +~~~ + +#### `print!` + +This and `println` emit their output to stdout. Similarly to the `write!` macro, +the goal of these macros is to avoid intermediate allocations when printing +output. Example usage is: + +~~~{.rust} +print!("Hello {}!", "world"); +println!("I have a newline {}", "character at the end"); +~~~ + +#### `format_args!` +This is a curious macro which is used to safely pass around +an opaque object describing the format string. This object +does not require any heap allocations to create, and it only +references information on the stack. Under the hood, all of +the related macros are implemented in terms of this. First +off, some example usage is: + +~~~{.rust} +use std::fmt; + +format_args!(fmt::format, "this returns {}", "~str"); +format_args!(|args| { fmt::write(my_writer, args) }, "some {}", "args"); +format_args!(my_fn, "format {}", "string"); +~~~ + +The first argument of the `format_args!` macro is a function (or closure) which +takes one argument of type `&fmt::Arguments`. This structure can then be +passed to the `write` and `format` functions inside this module in order to +process the format string. The goal of this macro is to even further prevent +intermediate allocations when dealing formatting strings. + +For example, a logging library could use the standard formatting syntax, but it +would internally pass around this structure until it has been determined where +output should go to. + +It is unsafe to programmatically create an instance of `fmt::Arguments` because +the operations performed when executing a format string require the compile-time +checks provided by the compiler. The `format_args!` macro is the only method of +safely creating these structures, but they can be unsafely created with the +constructor provided. + ## Internationalization The formatting syntax supported by the `format!` extension supports @@ -163,7 +235,7 @@ Furthermore, whenever a case is running, the special character `#` can be used to reference the string value of the argument which was selected upon. As an example: -~~~ +~~~{.rust} format!("{0, select, other{#}}", "hello") // => ~"hello" ~~~ diff --git a/src/test/compile-fail/ifmt-bad-arg.rs b/src/test/compile-fail/ifmt-bad-arg.rs index 35085feaf1505..f9552725af345 100644 --- a/src/test/compile-fail/ifmt-bad-arg.rs +++ b/src/test/compile-fail/ifmt-bad-arg.rs @@ -72,8 +72,7 @@ fn main() { format!("foo } bar"); //~ ERROR: unmatched `}` found format!("foo }"); //~ ERROR: unmatched `}` found - // FIXME(#5794) the spans on these errors are pretty terrible - //format!(); - //format!("" 1); - //format!("", 1 1); + format!(); //~ ERROR: requires at least a format string argument + format!("" 1); //~ ERROR: expected token: `,` + format!("", 1 1); //~ ERROR: expected token: `,` } From 640613892fc5ab055853b48934b6e4ecf895c2dd Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sun, 15 Sep 2013 01:27:38 -0700 Subject: [PATCH 3/3] Fix expand_stmt as well as expand_expr to use the correct span The same fix as before is still relevant, I just forgot to update the expand_stmt macro expansion site. The tests for format!() suffice as tests for this change. --- src/libsyntax/ext/expand.rs | 40 +++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index e4c69c1973aaa..ac094c27a8119 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -81,20 +81,10 @@ pub fn expand_expr(extsbox: @mut SyntaxEnv, // be the root of the call stack. That's the most // relevant span and it's the actual invocation of // the macro. - let mut relevant_info = cx.backtrace(); - let mut einfo = relevant_info.unwrap(); - loop { - match relevant_info { - None => { break } - Some(e) => { - einfo = e; - relevant_info = einfo.call_site.expn_info; - } - } - } + let mac_span = original_span(cx); let expanded = - match expandfun(cx, einfo.call_site, + match expandfun(cx, mac_span.call_site, marked_before, marked_ctxt) { MRExpr(e) => e, MRAny(expr_maker,_,_) => expr_maker(), @@ -400,11 +390,11 @@ pub fn expand_stmt(extsbox: @mut SyntaxEnv, -> (Option, Span) { // why the copying here and not in expand_expr? // looks like classic changed-in-only-one-place - let (mac, pth, tts, semi, ctxt) = match *s { + let (pth, tts, semi, ctxt) = match *s { StmtMac(ref mac, semi) => { match mac.node { mac_invoc_tt(ref pth, ref tts, ctxt) => { - ((*mac).clone(), pth, (*tts).clone(), semi, ctxt) + (pth, (*tts).clone(), semi, ctxt) } } } @@ -431,7 +421,13 @@ pub fn expand_stmt(extsbox: @mut SyntaxEnv, // mark before expansion: let marked_tts = mark_tts(tts,fm); let marked_ctxt = new_mark(fm,ctxt); - let expanded = match expandfun(cx, mac.span, marked_tts, marked_ctxt) { + + // See the comment in expand_expr for why we want the original span, + // not the current mac.span. + let mac_span = original_span(cx); + + let expanded = match expandfun(cx, mac_span.call_site, + marked_tts, marked_ctxt) { MRExpr(e) => @codemap::Spanned { node: StmtExpr(e, ast::DUMMY_NODE_ID), span: e.span}, @@ -1270,6 +1266,20 @@ pub fn mtwt_cancel_outer_mark(tts: &[ast::token_tree], ctxt: ast::SyntaxContext) mark_tts(tts,outer_mark) } +fn original_span(cx: @ExtCtxt) -> @codemap::ExpnInfo { + let mut relevant_info = cx.backtrace(); + let mut einfo = relevant_info.unwrap(); + loop { + match relevant_info { + None => { break } + Some(e) => { + einfo = e; + relevant_info = einfo.call_site.expn_info; + } + } + } + return einfo; +} #[cfg(test)] mod test {