From fdef4f185ea6a1b560c1370c10ee561135af483d Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Wed, 1 Jan 2020 13:14:33 -0500 Subject: [PATCH 1/4] Delete unused "next" variants from formatting infrastructure The formatting infrastructure stopped emitting these a while back, and in removing them we can simplify related code. --- src/libcore/fmt/mod.rs | 19 +++++++++++-------- src/libcore/fmt/rt/v1.rs | 6 ++++-- src/librustc_builtin_macros/format.rs | 13 +------------ 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/libcore/fmt/mod.rs b/src/libcore/fmt/mod.rs index e68f3c58a3e..64c270522d3 100644 --- a/src/libcore/fmt/mod.rs +++ b/src/libcore/fmt/mod.rs @@ -10,7 +10,6 @@ use crate::mem; use crate::num::flt2dec; use crate::ops::Deref; use crate::result; -use crate::slice; use crate::str; mod builders; @@ -234,7 +233,6 @@ pub struct Formatter<'a> { precision: Option, buf: &'a mut (dyn Write + 'a), - curarg: slice::Iter<'a, ArgumentV1<'a>>, args: &'a [ArgumentV1<'a>], } @@ -1044,7 +1042,6 @@ pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result { align: rt::v1::Alignment::Unknown, fill: ' ', args: args.args, - curarg: args.args.iter(), }; let mut idx = 0; @@ -1117,7 +1114,6 @@ impl<'a> Formatter<'a> { // These only exist in the struct for the `run` method, // which won’t be used together with this method. - curarg: self.curarg.clone(), args: self.args, } } @@ -1134,9 +1130,17 @@ impl<'a> Formatter<'a> { self.precision = self.getcount(&arg.format.precision); // Extract the correct argument - let value = match arg.position { - rt::v1::Position::Next => *self.curarg.next().unwrap(), - rt::v1::Position::At(i) => self.args[i], + let value = { + #[cfg(bootstrap)] + { + match arg.position { + rt::v1::Position::At(i) => self.args[i], + } + } + #[cfg(not(bootstrap))] + { + self.args[arg.position] + } }; // Then actually do some printing @@ -1148,7 +1152,6 @@ impl<'a> Formatter<'a> { rt::v1::Count::Is(n) => Some(n), rt::v1::Count::Implied => None, rt::v1::Count::Param(i) => self.args[i].as_usize(), - rt::v1::Count::NextParam => self.curarg.next()?.as_usize(), } } diff --git a/src/libcore/fmt/rt/v1.rs b/src/libcore/fmt/rt/v1.rs index 826ae36d2d1..fd81f93242b 100644 --- a/src/libcore/fmt/rt/v1.rs +++ b/src/libcore/fmt/rt/v1.rs @@ -7,7 +7,10 @@ #[derive(Copy, Clone)] pub struct Argument { + #[cfg(bootstrap)] pub position: Position, + #[cfg(not(bootstrap))] + pub position: usize, pub format: FormatSpec, } @@ -37,12 +40,11 @@ pub enum Alignment { pub enum Count { Is(usize), Param(usize), - NextParam, Implied, } +#[cfg(bootstrap)] #[derive(Copy, Clone)] pub enum Position { - Next, At(usize), } diff --git a/src/librustc_builtin_macros/format.rs b/src/librustc_builtin_macros/format.rs index 6fca74e2239..3f4e24ca993 100644 --- a/src/librustc_builtin_macros/format.rs +++ b/src/librustc_builtin_macros/format.rs @@ -590,17 +590,6 @@ impl<'a, 'b> Context<'a, 'b> { parse::NextArgument(ref arg) => { // Build the position let pos = { - let pos = |c, arg| { - let mut path = Context::rtpath(self.ecx, "Position"); - path.push(self.ecx.ident_of(c, sp)); - match arg { - Some(i) => { - let arg = self.ecx.expr_usize(sp, i); - self.ecx.expr_call_global(sp, path, vec![arg]) - } - None => self.ecx.expr_path(self.ecx.path_global(sp, path)), - } - }; match arg.position { parse::ArgumentIs(i) | parse::ArgumentImplicitlyIs(i) => { // Map to index in final generated argument array @@ -615,7 +604,7 @@ impl<'a, 'b> Context<'a, 'b> { arg_idx } }; - pos("At", Some(arg_idx)) + self.ecx.expr_usize(sp, arg_idx) } // should never be the case, because names are already From 4919b96f81d9df0a7c0fe83ad5d66cef18ddfcfb Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Wed, 1 Jan 2020 13:58:57 -0500 Subject: [PATCH 2/4] Move run/getcount to functions These are only called from one place and don't generally support being called from other places; furthermore, they're the only formatter functions that look at the `args` field (which a future commit will remove). --- src/libcore/fmt/mod.rs | 73 ++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 38 deletions(-) diff --git a/src/libcore/fmt/mod.rs b/src/libcore/fmt/mod.rs index 64c270522d3..e76a8490f51 100644 --- a/src/libcore/fmt/mod.rs +++ b/src/libcore/fmt/mod.rs @@ -1060,7 +1060,7 @@ pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result { // a string piece. for (arg, piece) in fmt.iter().zip(args.pieces.iter()) { formatter.buf.write_str(*piece)?; - formatter.run(arg)?; + run(&mut formatter, arg)?; idx += 1; } } @@ -1074,6 +1074,40 @@ pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result { Ok(()) } +fn run(fmt: &mut Formatter<'_>, arg: &rt::v1::Argument) -> Result { + // Fill in the format parameters into the formatter + fmt.fill = arg.format.fill; + fmt.align = arg.format.align; + fmt.flags = arg.format.flags; + fmt.width = getcount(&fmt.args, &arg.format.width); + fmt.precision = getcount(&fmt.args, &arg.format.precision); + + // Extract the correct argument + let value = { + #[cfg(bootstrap)] + { + match arg.position { + rt::v1::Position::At(i) => fmt.args[i], + } + } + #[cfg(not(bootstrap))] + { + fmt.args[arg.position] + } + }; + + // Then actually do some printing + (value.formatter)(value.value, fmt) +} + +fn getcount(args: &[ArgumentV1<'_>], cnt: &rt::v1::Count) -> Option { + match *cnt { + rt::v1::Count::Is(n) => Some(n), + rt::v1::Count::Implied => None, + rt::v1::Count::Param(i) => args[i].as_usize(), + } +} + /// Padding after the end of something. Returned by `Formatter::padding`. #[must_use = "don't forget to write the post padding"] struct PostPadding { @@ -1118,43 +1152,6 @@ impl<'a> Formatter<'a> { } } - // First up is the collection of functions used to execute a format string - // at runtime. This consumes all of the compile-time statics generated by - // the format! syntax extension. - fn run(&mut self, arg: &rt::v1::Argument) -> Result { - // Fill in the format parameters into the formatter - self.fill = arg.format.fill; - self.align = arg.format.align; - self.flags = arg.format.flags; - self.width = self.getcount(&arg.format.width); - self.precision = self.getcount(&arg.format.precision); - - // Extract the correct argument - let value = { - #[cfg(bootstrap)] - { - match arg.position { - rt::v1::Position::At(i) => self.args[i], - } - } - #[cfg(not(bootstrap))] - { - self.args[arg.position] - } - }; - - // Then actually do some printing - (value.formatter)(value.value, self) - } - - fn getcount(&mut self, cnt: &rt::v1::Count) -> Option { - match *cnt { - rt::v1::Count::Is(n) => Some(n), - rt::v1::Count::Implied => None, - rt::v1::Count::Param(i) => self.args[i].as_usize(), - } - } - // Helper methods used for padding and processing formatting arguments that // all formatting traits can use. From 9ae32c9b27e5c13e6903c21856a403ec7067cadd Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Wed, 1 Jan 2020 14:09:50 -0500 Subject: [PATCH 3/4] Drop args from Formatter These are no longer used by Formatter methods. --- src/libcore/fmt/mod.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/libcore/fmt/mod.rs b/src/libcore/fmt/mod.rs index e76a8490f51..900ef63f1df 100644 --- a/src/libcore/fmt/mod.rs +++ b/src/libcore/fmt/mod.rs @@ -233,7 +233,6 @@ pub struct Formatter<'a> { precision: Option, buf: &'a mut (dyn Write + 'a), - args: &'a [ArgumentV1<'a>], } // NB. Argument is essentially an optimized partially applied formatting function, @@ -1041,7 +1040,6 @@ pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result { buf: output, align: rt::v1::Alignment::Unknown, fill: ' ', - args: args.args, }; let mut idx = 0; @@ -1060,7 +1058,7 @@ pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result { // a string piece. for (arg, piece) in fmt.iter().zip(args.pieces.iter()) { formatter.buf.write_str(*piece)?; - run(&mut formatter, arg)?; + run(&mut formatter, arg, &args.args)?; idx += 1; } } @@ -1074,25 +1072,24 @@ pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result { Ok(()) } -fn run(fmt: &mut Formatter<'_>, arg: &rt::v1::Argument) -> Result { - // Fill in the format parameters into the formatter +fn run(fmt: &mut Formatter<'_>, arg: &rt::v1::Argument, args: &[ArgumentV1<'_>]) -> Result { fmt.fill = arg.format.fill; fmt.align = arg.format.align; fmt.flags = arg.format.flags; - fmt.width = getcount(&fmt.args, &arg.format.width); - fmt.precision = getcount(&fmt.args, &arg.format.precision); + fmt.width = getcount(args, &arg.format.width); + fmt.precision = getcount(args, &arg.format.precision); // Extract the correct argument let value = { #[cfg(bootstrap)] { match arg.position { - rt::v1::Position::At(i) => fmt.args[i], + rt::v1::Position::At(i) => args[i], } } #[cfg(not(bootstrap))] { - fmt.args[arg.position] + args[arg.position] } }; @@ -1145,10 +1142,6 @@ impl<'a> Formatter<'a> { align: self.align, width: self.width, precision: self.precision, - - // These only exist in the struct for the `run` method, - // which won’t be used together with this method. - args: self.args, } } From a804a455289cfa6d39ec92903959c4614c48d080 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Mon, 20 Jan 2020 12:17:12 -0500 Subject: [PATCH 4/4] Fix UI test fmt::Formatter is still not Send/Sync, but the UI test emitted two errors, for the dyn Write and the Void inside Formatter. As of this PR, the Void is now gone, but the dyn Write remains. --- src/test/ui/async-await/async-fn-nonsend.rs | 15 ++++++------ .../ui/async-await/async-fn-nonsend.stderr | 24 +------------------ 2 files changed, 8 insertions(+), 31 deletions(-) diff --git a/src/test/ui/async-await/async-fn-nonsend.rs b/src/test/ui/async-await/async-fn-nonsend.rs index 645c903c6ba..ceeebbca519 100644 --- a/src/test/ui/async-await/async-fn-nonsend.rs +++ b/src/test/ui/async-await/async-fn-nonsend.rs @@ -2,15 +2,15 @@ // edition:2018 // compile-flags: --crate-type lib -use std::{ - cell::RefCell, - fmt::Debug, - rc::Rc, -}; +use std::{cell::RefCell, fmt::Debug, rc::Rc}; -fn non_sync() -> impl Debug { RefCell::new(()) } +fn non_sync() -> impl Debug { + RefCell::new(()) +} -fn non_send() -> impl Debug { Rc::new(()) } +fn non_send() -> impl Debug { + Rc::new(()) +} fn take_ref(_: &T) {} @@ -53,5 +53,4 @@ pub fn pass_assert() { //~^ ERROR future cannot be sent between threads safely assert_send(non_sync_with_method_call()); //~^ ERROR future cannot be sent between threads safely - //~^^ ERROR future cannot be sent between threads safely } diff --git a/src/test/ui/async-await/async-fn-nonsend.stderr b/src/test/ui/async-await/async-fn-nonsend.stderr index 5c870ca2d02..105fd23ecfb 100644 --- a/src/test/ui/async-await/async-fn-nonsend.stderr +++ b/src/test/ui/async-await/async-fn-nonsend.stderr @@ -62,27 +62,5 @@ LL | } LL | } | - `f` is later dropped here -error: future cannot be sent between threads safely - --> $DIR/async-fn-nonsend.rs:54:5 - | -LL | fn assert_send(_: impl Send) {} - | ----------- ---- required by this bound in `assert_send` -... -LL | assert_send(non_sync_with_method_call()); - | ^^^^^^^^^^^ future returned by `non_sync_with_method_call` is not `Send` - | - = help: within `std::fmt::ArgumentV1<'_>`, the trait `std::marker::Sync` is not implemented for `*mut (dyn std::ops::Fn() + 'static)` -note: future is not `Send` as this value is used across an await - --> $DIR/async-fn-nonsend.rs:43:9 - | -LL | let f: &mut std::fmt::Formatter = panic!(); - | - has type `&mut std::fmt::Formatter<'_>` -LL | if non_sync().fmt(f).unwrap() == () { -LL | fut().await; - | ^^^^^^^^^^^ await occurs here, with `f` maybe used later -LL | } -LL | } - | - `f` is later dropped here - -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors