From 159dcb2194e6af28e070f15b9a69af354f87d6d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 29 Jul 2019 18:19:21 -0700 Subject: [PATCH 1/5] On `format!()` arg count mismatch provide extra info When positional width and precision formatting flags are present in a formatting string that has an argument count mismatch, provide extra information pointing at them making it easiser to understand where the problem may lay: ``` error: 4 positional arguments in format string, but there are 3 arguments --> $DIR/ifmt-bad-arg.rs:78:15 | LL | println!("{} {:.*} {}", 1, 3.2, 4); | ^^ ^^--^ ^^ --- this parameter corresponds to the precision flag | | | this precision flag adds an extra required argument at position 1, which is why there are 4 arguments expected | = note: positional arguments are zero-based = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html error: 4 positional arguments in format string, but there are 3 arguments --> $DIR/ifmt-bad-arg.rs:81:15 | LL | println!("{} {:07$.*} {}", 1, 3.2, 4); | ^^ ^^-----^ ^^ --- this parameter corresponds to the precision flag | | | | | this precision flag adds an extra required argument at position 1, which is why there are 4 arguments expected | this width flag expects an `usize` argument at position 7, but there are 3 arguments | = note: positional arguments are zero-based = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html error: 3 positional arguments in format string, but there are 3 arguments --> $DIR/ifmt-bad-arg.rs:84:15 | LL | println!("{} {:07$} {}", 1, 3.2, 4); | ^^ ^^---^ ^^ | | | this width flag expects an `usize` argument at position 7, but there are 3 arguments | = note: positional arguments are zero-based = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html ``` --- src/libfmt_macros/lib.rs | 76 +++++++++++++------- src/libsyntax_ext/format.rs | 107 +++++++++++++++++++++++++---- src/test/ui/if/ifmt-bad-arg.rs | 8 +++ src/test/ui/if/ifmt-bad-arg.stderr | 55 ++++++++++++++- 4 files changed, 206 insertions(+), 40 deletions(-) diff --git a/src/libfmt_macros/lib.rs b/src/libfmt_macros/lib.rs index f1c2b1fb871..b01c3a4aa7f 100644 --- a/src/libfmt_macros/lib.rs +++ b/src/libfmt_macros/lib.rs @@ -59,16 +59,20 @@ pub struct Argument<'a> { /// Specification for the formatting of an argument in the format string. #[derive(Copy, Clone, PartialEq)] pub struct FormatSpec<'a> { - /// Optionally specified character to fill alignment with + /// Optionally specified character to fill alignment with. pub fill: Option, - /// Optionally specified alignment + /// Optionally specified alignment. pub align: Alignment, - /// Packed version of various flags provided + /// Packed version of various flags provided. pub flags: u32, - /// The integer precision to use + /// The integer precision to use. pub precision: Count, - /// The string width requested for the resulting format + /// The span of the precision formatting flag (for diagnostics). + pub precision_span: Option, + /// The string width requested for the resulting format. pub width: Count, + /// The span of the width formatting flag (for diagnostics). + pub width_span: Option, /// The descriptor string representing the name of the format desired for /// this argument, this can be empty or any number of characters, although /// it is required to be one word. @@ -285,19 +289,24 @@ impl<'a> Parser<'a> { } /// Optionally consumes the specified character. If the character is not at - /// the current position, then the current iterator isn't moved and false is - /// returned, otherwise the character is consumed and true is returned. + /// the current position, then the current iterator isn't moved and `false` is + /// returned, otherwise the character is consumed and `true` is returned. fn consume(&mut self, c: char) -> bool { - if let Some(&(_, maybe)) = self.cur.peek() { + self.consume_pos(c).is_some() + } + + /// Optionally consumes the specified character. If the character is not at + /// the current position, then the current iterator isn't moved and `None` is + /// returned, otherwise the character is consumed and the current position is + /// returned. + fn consume_pos(&mut self, c: char) -> Option { + if let Some(&(pos, maybe)) = self.cur.peek() { if c == maybe { self.cur.next(); - true - } else { - false + return Some(pos); } - } else { - false } + None } fn to_span_index(&self, pos: usize) -> InnerOffset { @@ -465,7 +474,9 @@ impl<'a> Parser<'a> { align: AlignUnknown, flags: 0, precision: CountImplied, + precision_span: None, width: CountImplied, + width_span: None, ty: &self.input[..0], }; if !self.consume(':') { @@ -502,6 +513,11 @@ impl<'a> Parser<'a> { } // Width and precision let mut havewidth = false; + + let mut width_span_start = 0; + if let Some((pos, '0')) = self.cur.peek() { + width_span_start = *pos; + } if self.consume('0') { // small ambiguity with '0$' as a format string. In theory this is a // '0' flag and then an ill-formatted format string with just a '$' @@ -515,17 +531,28 @@ impl<'a> Parser<'a> { } } if !havewidth { - spec.width = self.count(); + if width_span_start == 0 { + if let Some((pos, _)) = self.cur.peek() { + width_span_start = *pos; + } + } + let (w, sp) = self.count(width_span_start); + spec.width = w; + spec.width_span = sp; } - if self.consume('.') { - if self.consume('*') { + if let Some(start) = self.consume_pos('.') { + if let Some(end) = self.consume_pos('*') { // Resolve `CountIsNextParam`. // We can do this immediately as `position` is resolved later. let i = self.curarg; self.curarg += 1; spec.precision = CountIsParam(i); + spec.precision_span = + Some(self.to_span_index(start).to(self.to_span_index(end + 1))); } else { - spec.precision = self.count(); + let (p, sp) = self.count(start); + spec.precision = p; + spec.precision_span = sp; } } // Optional radix followed by the actual format specifier @@ -554,24 +581,25 @@ impl<'a> Parser<'a> { /// Parses a Count parameter at the current position. This does not check /// for 'CountIsNextParam' because that is only used in precision, not /// width. - fn count(&mut self) -> Count { + fn count(&mut self, start: usize) -> (Count, Option) { if let Some(i) = self.integer() { - if self.consume('$') { - CountIsParam(i) + if let Some(end) = self.consume_pos('$') { + let span = self.to_span_index(start).to(self.to_span_index(end + 1)); + (CountIsParam(i), Some(span)) } else { - CountIs(i) + (CountIs(i), None) } } else { let tmp = self.cur.clone(); let word = self.word(); if word.is_empty() { self.cur = tmp; - CountImplied + (CountImplied, None) } else if self.consume('$') { - CountIsName(Symbol::intern(word)) + (CountIsName(Symbol::intern(word)), None) } else { self.cur = tmp; - CountImplied + (CountImplied, None) } } } diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index e53660b6568..aa8fc70d806 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -109,6 +109,8 @@ struct Context<'a, 'b> { invalid_refs: Vec<(usize, usize)>, /// Spans of all the formatting arguments, in order. arg_spans: Vec, + /// All the formatting arguments that have formatting flags set, in order for diagnostics. + arg_with_formatting: Vec>, /// Whether this formatting string is a literal or it comes from a macro. is_literal: bool, } @@ -279,14 +281,20 @@ impl<'a, 'b> Context<'a, 'b> { .iter() .map(|(r, pos)| (r.to_string(), self.arg_spans.get(*pos))); + let mut zero_based_note = false; + if self.names.is_empty() && !numbered_position_args { + let count = self.pieces.len() + self.arg_with_formatting + .iter() + .filter(|fmt| fmt.precision_span.is_some()) + .count(); e = self.ecx.mut_span_err( sp, &format!( "{} positional argument{} in format string, but {}", - self.pieces.len(), - if self.pieces.len() > 1 { "s" } else { "" }, - self.describe_num_args() + count, + if count > 1 { "s" } else { "" }, + self.describe_num_args(), ), ); } else { @@ -317,9 +325,70 @@ impl<'a, 'b> Context<'a, 'b> { &format!("invalid reference to positional {} ({})", arg_list, self.describe_num_args())); - e.note("positional arguments are zero-based"); + zero_based_note = true; }; + for fmt in &self.arg_with_formatting { + if let Some(span) = fmt.precision_span { + let span = self.fmtsp.from_inner(span); + match fmt.precision { + parse::CountIsParam(pos) if pos > self.args.len() => { + e.span_label(span, &format!( + "this precision flag expects an `usize` argument at position {}, \ + but {}", + pos, + self.describe_num_args(), + )); + zero_based_note = true; + } + parse::CountIsParam(pos) => { + let count = self.pieces.len() + self.arg_with_formatting + .iter() + .filter(|fmt| fmt.precision_span.is_some()) + .count(); + e.span_label(span, &format!( + "this precision flag adds an extra required argument at position {}, \ + which is why there {} expected", + pos, + if count == 1 { + "is 1 argument".to_string() + } else { + format!("are {} arguments", count) + }, + )); + e.span_label( + self.args[pos].span, + "this parameter corresponds to the precision flag", + ); + zero_based_note = true; + } + _ => {} + } + } + if let Some(span) = fmt.width_span { + let span = self.fmtsp.from_inner(span); + match fmt.width { + parse::CountIsParam(pos) if pos > self.args.len() => { + e.span_label(span, &format!( + "this width flag expects an `usize` argument at position {}, \ + but {}", + pos, + self.describe_num_args(), + )); + zero_based_note = true; + } + _ => {} + } + } + } + if zero_based_note { + e.note("positional arguments are zero-based"); + } + if !self.arg_with_formatting.is_empty() { + e.note("for information about formatting flags, visit \ + https://doc.rust-lang.org/std/fmt/index.html"); + } + e.emit(); } @@ -435,10 +504,11 @@ impl<'a, 'b> Context<'a, 'b> { /// Builds a static `rt::Argument` from a `parse::Piece` or append /// to the `literal` string. - fn build_piece(&mut self, - piece: &parse::Piece<'_>, - arg_index_consumed: &mut Vec) - -> Option> { + fn build_piece( + &mut self, + piece: &parse::Piece<'a>, + arg_index_consumed: &mut Vec, + ) -> Option> { let sp = self.macsp; match *piece { parse::String(s) => { @@ -496,7 +566,9 @@ impl<'a, 'b> Context<'a, 'b> { align: parse::AlignUnknown, flags: 0, precision: parse::CountImplied, + precision_span: None, width: parse::CountImplied, + width_span: None, ty: arg.format.ty, }, }; @@ -506,6 +578,9 @@ impl<'a, 'b> Context<'a, 'b> { let pos_simple = arg.position.index() == simple_arg.position.index(); + if arg.format.precision_span.is_some() || arg.format.width_span.is_some() { + self.arg_with_formatting.push(arg.format); //'liself.fmtsp.from_inner(span)); + } if !pos_simple || arg.format != simple_arg.format || fill != ' ' { self.all_pieces_simple = false; } @@ -530,7 +605,7 @@ impl<'a, 'b> Context<'a, 'b> { let path = self.ecx.path_global(sp, Context::rtpath(self.ecx, "FormatSpec")); let fmt = self.ecx.expr_struct( sp, - path, + path, vec![ self.ecx.field_imm(sp, self.ecx.ident_of("fill"), fill), self.ecx.field_imm(sp, self.ecx.ident_of("align"), align), @@ -657,12 +732,13 @@ impl<'a, 'b> Context<'a, 'b> { self.ecx.expr_call_global(self.macsp, path, fn_args) } - fn format_arg(ecx: &ExtCtxt<'_>, - macsp: Span, - mut sp: Span, - ty: &ArgumentType, - arg: ast::Ident) - -> P { + fn format_arg( + ecx: &ExtCtxt<'_>, + macsp: Span, + mut sp: Span, + ty: &ArgumentType, + arg: ast::Ident, + ) -> P { sp = sp.apply_mark(ecx.current_expansion.id); let arg = ecx.expr_ident(sp, arg); let trait_ = match *ty { @@ -941,6 +1017,7 @@ pub fn expand_preparsed_format_args( fmtsp: fmt.span, invalid_refs: Vec::new(), arg_spans, + arg_with_formatting: Vec::new(), is_literal, }; diff --git a/src/test/ui/if/ifmt-bad-arg.rs b/src/test/ui/if/ifmt-bad-arg.rs index 0ebe1fa2dff..d5c7950542d 100644 --- a/src/test/ui/if/ifmt-bad-arg.rs +++ b/src/test/ui/if/ifmt-bad-arg.rs @@ -75,4 +75,12 @@ ninth number: { tenth number: {}", 1, 2, 3, 4, 5, 6, 7, 8, 9, 10); //~^^ ERROR: invalid format string + println!("{} {:.*} {}", 1, 3.2, 4); + //~^ ERROR 4 positional arguments in format string, but there are 3 arguments + //~| ERROR mismatched types + println!("{} {:07$.*} {}", 1, 3.2, 4); + //~^ ERROR 4 positional arguments in format string, but there are 3 arguments + //~| ERROR mismatched types + println!("{} {:07$} {}", 1, 3.2, 4); + //~^ ERROR 3 positional arguments in format string, but there are 3 arguments } diff --git a/src/test/ui/if/ifmt-bad-arg.stderr b/src/test/ui/if/ifmt-bad-arg.stderr index 835b5b6592b..1eaeb017587 100644 --- a/src/test/ui/if/ifmt-bad-arg.stderr +++ b/src/test/ui/if/ifmt-bad-arg.stderr @@ -220,5 +220,58 @@ LL | tenth number: {}", | = note: if you intended to print `{`, you can escape it using `{{` -error: aborting due to 28 previous errors +error: 4 positional arguments in format string, but there are 3 arguments + --> $DIR/ifmt-bad-arg.rs:78:15 + | +LL | println!("{} {:.*} {}", 1, 3.2, 4); + | ^^ ^^--^ ^^ --- this parameter corresponds to the precision flag + | | + | this precision flag adds an extra required argument at position 1, which is why there are 4 arguments expected + | + = note: positional arguments are zero-based + = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html +error: 4 positional arguments in format string, but there are 3 arguments + --> $DIR/ifmt-bad-arg.rs:81:15 + | +LL | println!("{} {:07$.*} {}", 1, 3.2, 4); + | ^^ ^^-----^ ^^ --- this parameter corresponds to the precision flag + | | | + | | this precision flag adds an extra required argument at position 1, which is why there are 4 arguments expected + | this width flag expects an `usize` argument at position 7, but there are 3 arguments + | + = note: positional arguments are zero-based + = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html + +error: 3 positional arguments in format string, but there are 3 arguments + --> $DIR/ifmt-bad-arg.rs:84:15 + | +LL | println!("{} {:07$} {}", 1, 3.2, 4); + | ^^ ^^---^ ^^ + | | + | this width flag expects an `usize` argument at position 7, but there are 3 arguments + | + = note: positional arguments are zero-based + = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html + +error[E0308]: mismatched types + --> $DIR/ifmt-bad-arg.rs:78:32 + | +LL | println!("{} {:.*} {}", 1, 3.2, 4); + | ^^^ expected usize, found floating-point number + | + = note: expected type `&usize` + found type `&{float}` + +error[E0308]: mismatched types + --> $DIR/ifmt-bad-arg.rs:81:35 + | +LL | println!("{} {:07$.*} {}", 1, 3.2, 4); + | ^^^ expected usize, found floating-point number + | + = note: expected type `&usize` + found type `&{float}` + +error: aborting due to 33 previous errors + +For more information about this error, try `rustc --explain E0308`. From 86f4f68b70fb743d05778e88a44bb1259a03e3df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 29 Jul 2019 21:24:10 -0700 Subject: [PATCH 2/5] Improve handling of invalid references in `format!()` --- src/libsyntax_ext/format.rs | 47 +++++++++++++++++------- src/test/ui/if/ifmt-bad-arg.rs | 5 ++- src/test/ui/if/ifmt-bad-arg.stderr | 37 +++++++++++++++++-- src/test/ui/if/ifmt-unknown-trait.stderr | 11 ++++++ 4 files changed, 82 insertions(+), 18 deletions(-) diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index aa8fc70d806..3e0588d03eb 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -275,19 +275,18 @@ impl<'a, 'b> Context<'a, 'b> { } else { MultiSpan::from_span(self.fmtsp) }; - let refs_len = self.invalid_refs.len(); - let mut refs = self + let refs = self .invalid_refs .iter() .map(|(r, pos)| (r.to_string(), self.arg_spans.get(*pos))); let mut zero_based_note = false; - if self.names.is_empty() && !numbered_position_args { - let count = self.pieces.len() + self.arg_with_formatting - .iter() - .filter(|fmt| fmt.precision_span.is_some()) - .count(); + let count = self.pieces.len() + self.arg_with_formatting + .iter() + .filter(|fmt| fmt.precision_span.is_some()) + .count(); + if self.names.is_empty() && !numbered_position_args && count != self.args.len() { e = self.ecx.mut_span_err( sp, &format!( @@ -298,14 +297,22 @@ impl<'a, 'b> Context<'a, 'b> { ), ); } else { - let (arg_list, mut sp) = if refs_len == 1 { - let (reg, pos) = refs.next().unwrap(); + let (mut refs, spans): (Vec<_>, Vec<_>) = refs.unzip(); + // Avoid `invalid reference to positional arguments 7 and 7 (there is 1 argument)` + // for `println!("{7:7$}", 1);` + refs.dedup(); + refs.sort(); + let (arg_list, mut sp) = if refs.len() == 1 { + let spans: Vec<_> = spans.into_iter().filter_map(|sp| sp.map(|sp| *sp)).collect(); ( - format!("argument {}", reg), - MultiSpan::from_span(*pos.unwrap_or(&self.fmtsp)), + format!("argument {}", refs[0]), + if spans.is_empty() { + MultiSpan::from_span(self.fmtsp) + } else { + MultiSpan::from_spans(spans) + }, ) } else { - let (mut refs, spans): (Vec<_>, Vec<_>) = refs.unzip(); let pos = MultiSpan::from_spans(spans.into_iter().map(|s| *s.unwrap()).collect()); let reg = refs.pop().unwrap(); ( @@ -754,7 +761,21 @@ impl<'a, 'b> Context<'a, 'b> { "x" => "LowerHex", "X" => "UpperHex", _ => { - ecx.span_err(sp, &format!("unknown format trait `{}`", *tyname)); + let mut err = ecx.struct_span_err( + sp, + &format!("unknown format trait `{}`", *tyname), + ); + err.note("the only appropriate formatting traits are:\n\ + - ``, which uses the `Display` trait\n\ + - `?`, which uses the `Debug` trait\n\ + - `e`, which uses the `LowerExp` trait\n\ + - `E`, which uses the `UpperExp` trait\n\ + - `o`, which uses the `Octal` trait\n\ + - `p`, which uses the `Pointer` trait\n\ + - `b`, which uses the `Binary` trait\n\ + - `x`, which uses the `LowerHex` trait\n\ + - `X`, which uses the `UpperHex` trait"); + err.emit(); return DummyResult::raw_expr(sp, true); } } diff --git a/src/test/ui/if/ifmt-bad-arg.rs b/src/test/ui/if/ifmt-bad-arg.rs index d5c7950542d..ba897f171af 100644 --- a/src/test/ui/if/ifmt-bad-arg.rs +++ b/src/test/ui/if/ifmt-bad-arg.rs @@ -82,5 +82,8 @@ tenth number: {}", //~^ ERROR 4 positional arguments in format string, but there are 3 arguments //~| ERROR mismatched types println!("{} {:07$} {}", 1, 3.2, 4); - //~^ ERROR 3 positional arguments in format string, but there are 3 arguments + //~^ ERROR invalid reference to positional argument 7 (there are 3 arguments) + println!("{:foo}", 1); //~ ERROR unknown format trait `foo` + println!("{5} {:4$} {6:7$}", 1); + //~^ ERROR invalid reference to positional arguments 4, 5, 6 and 7 (there is 1 argument) } diff --git a/src/test/ui/if/ifmt-bad-arg.stderr b/src/test/ui/if/ifmt-bad-arg.stderr index 1eaeb017587..0de0c53f536 100644 --- a/src/test/ui/if/ifmt-bad-arg.stderr +++ b/src/test/ui/if/ifmt-bad-arg.stderr @@ -243,17 +243,46 @@ LL | println!("{} {:07$.*} {}", 1, 3.2, 4); = note: positional arguments are zero-based = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html -error: 3 positional arguments in format string, but there are 3 arguments - --> $DIR/ifmt-bad-arg.rs:84:15 +error: invalid reference to positional argument 7 (there are 3 arguments) + --> $DIR/ifmt-bad-arg.rs:84:18 | LL | println!("{} {:07$} {}", 1, 3.2, 4); - | ^^ ^^---^ ^^ + | ^^---^ | | | this width flag expects an `usize` argument at position 7, but there are 3 arguments | = note: positional arguments are zero-based = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html +error: unknown format trait `foo` + --> $DIR/ifmt-bad-arg.rs:86:24 + | +LL | println!("{:foo}", 1); + | ^ + | + = note: the only appropriate formatting traits are: + - ``, which uses the `Display` trait + - `?`, which uses the `Debug` trait + - `e`, which uses the `LowerExp` trait + - `E`, which uses the `UpperExp` trait + - `o`, which uses the `Octal` trait + - `p`, which uses the `Pointer` trait + - `b`, which uses the `Binary` trait + - `x`, which uses the `LowerHex` trait + - `X`, which uses the `UpperHex` trait + +error: invalid reference to positional arguments 4, 5, 6 and 7 (there is 1 argument) + --> $DIR/ifmt-bad-arg.rs:87:15 + | +LL | println!("{5} {:4$} {6:7$}", 1); + | ^^^ ^^--^ ^^^--^ + | | | + | | this width flag expects an `usize` argument at position 7, but there is 1 argument + | this width flag expects an `usize` argument at position 4, but there is 1 argument + | + = note: positional arguments are zero-based + = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html + error[E0308]: mismatched types --> $DIR/ifmt-bad-arg.rs:78:32 | @@ -272,6 +301,6 @@ LL | println!("{} {:07$.*} {}", 1, 3.2, 4); = note: expected type `&usize` found type `&{float}` -error: aborting due to 33 previous errors +error: aborting due to 35 previous errors For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/if/ifmt-unknown-trait.stderr b/src/test/ui/if/ifmt-unknown-trait.stderr index 9ea367c81ec..7853b5ca0c9 100644 --- a/src/test/ui/if/ifmt-unknown-trait.stderr +++ b/src/test/ui/if/ifmt-unknown-trait.stderr @@ -3,6 +3,17 @@ error: unknown format trait `notimplemented` | LL | format!("{:notimplemented}", "3"); | ^^^ + | + = note: the only appropriate formatting traits are: + - ``, which uses the `Display` trait + - `?`, which uses the `Debug` trait + - `e`, which uses the `LowerExp` trait + - `E`, which uses the `UpperExp` trait + - `o`, which uses the `Octal` trait + - `p`, which uses the `Pointer` trait + - `b`, which uses the `Binary` trait + - `x`, which uses the `LowerHex` trait + - `X`, which uses the `UpperHex` trait error: aborting due to previous error From 762f6452b96d93f4f108bf7101189f2688a9dcde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 29 Jul 2019 21:43:54 -0700 Subject: [PATCH 3/5] review comments --- src/libfmt_macros/lib.rs | 14 +++++--------- src/libsyntax_ext/format.rs | 2 +- src/libsyntax_pos/lib.rs | 1 + src/test/ui/if/ifmt-bad-arg.stderr | 14 +++++++------- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/libfmt_macros/lib.rs b/src/libfmt_macros/lib.rs index b01c3a4aa7f..4e16ad0ba0e 100644 --- a/src/libfmt_macros/lib.rs +++ b/src/libfmt_macros/lib.rs @@ -514,10 +514,6 @@ impl<'a> Parser<'a> { // Width and precision let mut havewidth = false; - let mut width_span_start = 0; - if let Some((pos, '0')) = self.cur.peek() { - width_span_start = *pos; - } if self.consume('0') { // small ambiguity with '0$' as a format string. In theory this is a // '0' flag and then an ill-formatted format string with just a '$' @@ -531,11 +527,11 @@ impl<'a> Parser<'a> { } } if !havewidth { - if width_span_start == 0 { - if let Some((pos, _)) = self.cur.peek() { - width_span_start = *pos; - } - } + let width_span_start = if let Some((pos, _)) = self.cur.peek() { + *pos + } else { + 0 + }; let (w, sp) = self.count(width_span_start); spec.width = w; spec.width_span = sp; diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index 3e0588d03eb..b9337fd9fab 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -586,7 +586,7 @@ impl<'a, 'b> Context<'a, 'b> { arg.position.index() == simple_arg.position.index(); if arg.format.precision_span.is_some() || arg.format.width_span.is_some() { - self.arg_with_formatting.push(arg.format); //'liself.fmtsp.from_inner(span)); + self.arg_with_formatting.push(arg.format); } if !pos_simple || arg.format != simple_arg.format || fill != ' ' { self.all_pieces_simple = false; diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs index e5f0892b37b..6a8fca2a28c 100644 --- a/src/libsyntax_pos/lib.rs +++ b/src/libsyntax_pos/lib.rs @@ -1402,6 +1402,7 @@ pub struct MalformedSourceMapPositions { pub end_pos: BytePos } +/// Range inside of a `Span` used for diagnostics when we only have access to relative positions. #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct InnerSpan { pub start: usize, diff --git a/src/test/ui/if/ifmt-bad-arg.stderr b/src/test/ui/if/ifmt-bad-arg.stderr index 0de0c53f536..336ea2254bf 100644 --- a/src/test/ui/if/ifmt-bad-arg.stderr +++ b/src/test/ui/if/ifmt-bad-arg.stderr @@ -235,10 +235,10 @@ error: 4 positional arguments in format string, but there are 3 arguments --> $DIR/ifmt-bad-arg.rs:81:15 | LL | println!("{} {:07$.*} {}", 1, 3.2, 4); - | ^^ ^^-----^ ^^ --- this parameter corresponds to the precision flag - | | | - | | this precision flag adds an extra required argument at position 1, which is why there are 4 arguments expected - | this width flag expects an `usize` argument at position 7, but there are 3 arguments + | ^^ ^^^----^ ^^ --- this parameter corresponds to the precision flag + | | | + | | this precision flag adds an extra required argument at position 1, which is why there are 4 arguments expected + | this width flag expects an `usize` argument at position 7, but there are 3 arguments | = note: positional arguments are zero-based = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html @@ -247,9 +247,9 @@ error: invalid reference to positional argument 7 (there are 3 arguments) --> $DIR/ifmt-bad-arg.rs:84:18 | LL | println!("{} {:07$} {}", 1, 3.2, 4); - | ^^---^ - | | - | this width flag expects an `usize` argument at position 7, but there are 3 arguments + | ^^^--^ + | | + | this width flag expects an `usize` argument at position 7, but there are 3 arguments | = note: positional arguments are zero-based = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html From 9e59d744acb256d908d9532c8c6dd4a60638f48b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 29 Jul 2019 21:51:32 -0700 Subject: [PATCH 4/5] fix tests --- src/libfmt_macros/tests.rs | 333 +++++++++++++++++++++---------------- 1 file changed, 187 insertions(+), 146 deletions(-) diff --git a/src/libfmt_macros/tests.rs b/src/libfmt_macros/tests.rs index 7282d4a5f24..e2ddb8810e9 100644 --- a/src/libfmt_macros/tests.rs +++ b/src/libfmt_macros/tests.rs @@ -12,6 +12,8 @@ fn fmtdflt() -> FormatSpec<'static> { flags: 0, precision: CountImplied, width: CountImplied, + precision_span: None, + width_span: None, ty: "", }; } @@ -79,165 +81,204 @@ fn format_position_nothing_else() { } #[test] fn format_type() { - same("{3:a}", - &[NextArgument(Argument { - position: ArgumentIs(3), - format: FormatSpec { - fill: None, - align: AlignUnknown, - flags: 0, - precision: CountImplied, - width: CountImplied, - ty: "a", - }, - })]); + same( + "{3:a}", + &[NextArgument(Argument { + position: ArgumentIs(3), + format: FormatSpec { + fill: None, + align: AlignUnknown, + flags: 0, + precision: CountImplied, + width: CountImplied, + precision_span: None, + width_span: None, + ty: "a", + }, + })]); } #[test] fn format_align_fill() { - same("{3:>}", - &[NextArgument(Argument { - position: ArgumentIs(3), - format: FormatSpec { - fill: None, - align: AlignRight, - flags: 0, - precision: CountImplied, - width: CountImplied, - ty: "", - }, - })]); - same("{3:0<}", - &[NextArgument(Argument { - position: ArgumentIs(3), - format: FormatSpec { - fill: Some('0'), - align: AlignLeft, - flags: 0, - precision: CountImplied, - width: CountImplied, - ty: "", - }, - })]); - same("{3:*}", + &[NextArgument(Argument { + position: ArgumentIs(3), + format: FormatSpec { + fill: None, + align: AlignRight, + flags: 0, + precision: CountImplied, + width: CountImplied, + precision_span: None, + width_span: None, + ty: "", + }, + })]); + same( + "{3:0<}", + &[NextArgument(Argument { + position: ArgumentIs(3), + format: FormatSpec { + fill: Some('0'), + align: AlignLeft, + flags: 0, + precision: CountImplied, + width: CountImplied, + precision_span: None, + width_span: None, + ty: "", + }, + })]); + same( + "{3:* Date: Wed, 31 Jul 2019 14:13:00 -0700 Subject: [PATCH 5/5] fix dedup --- src/libsyntax_ext/format.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index b9337fd9fab..fe9cad1e32f 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -300,8 +300,8 @@ impl<'a, 'b> Context<'a, 'b> { let (mut refs, spans): (Vec<_>, Vec<_>) = refs.unzip(); // Avoid `invalid reference to positional arguments 7 and 7 (there is 1 argument)` // for `println!("{7:7$}", 1);` - refs.dedup(); refs.sort(); + refs.dedup(); let (arg_list, mut sp) = if refs.len() == 1 { let spans: Vec<_> = spans.into_iter().filter_map(|sp| sp.map(|sp| *sp)).collect(); (