diff --git a/src/libfmt_macros/lib.rs b/src/libfmt_macros/lib.rs index 24430b2e377f..44cdb5e8a367 100644 --- a/src/libfmt_macros/lib.rs +++ b/src/libfmt_macros/lib.rs @@ -73,7 +73,9 @@ pub struct FormatSpec<'a> { /// Enum describing where an argument for a format can be located. #[derive(Copy, Clone, PartialEq)] pub enum Position<'a> { - /// The argument is located at a specific index. + /// The arugment is implied to be located at an index + ArgumentImplicitlyIs(usize), + /// The argument is located at a specific index given in the format ArgumentIs(usize), /// The argument has a name. ArgumentNamed(&'a str), @@ -275,7 +277,7 @@ impl<'a> Parser<'a> { None => { let i = self.curarg; self.curarg += 1; - ArgumentIs(i) + ArgumentImplicitlyIs(i) } }; @@ -517,7 +519,7 @@ mod tests { fn format_nothing() { same("{}", &[NextArgument(Argument { - position: ArgumentIs(0), + position: ArgumentImplicitlyIs(0), format: fmtdflt(), })]); } @@ -595,7 +597,7 @@ mod tests { fn format_counts() { same("{:10s}", &[NextArgument(Argument { - position: ArgumentIs(0), + position: ArgumentImplicitlyIs(0), format: FormatSpec { fill: None, align: AlignUnknown, @@ -607,7 +609,7 @@ mod tests { })]); same("{:10$.10s}", &[NextArgument(Argument { - position: ArgumentIs(0), + position: ArgumentImplicitlyIs(0), format: FormatSpec { fill: None, align: AlignUnknown, @@ -619,7 +621,7 @@ mod tests { })]); same("{:.*s}", &[NextArgument(Argument { - position: ArgumentIs(1), + position: ArgumentImplicitlyIs(1), format: FormatSpec { fill: None, align: AlignUnknown, @@ -631,7 +633,7 @@ mod tests { })]); same("{:.10$s}", &[NextArgument(Argument { - position: ArgumentIs(0), + position: ArgumentImplicitlyIs(0), format: FormatSpec { fill: None, align: AlignUnknown, @@ -643,7 +645,7 @@ mod tests { })]); same("{:a$.b$s}", &[NextArgument(Argument { - position: ArgumentIs(0), + position: ArgumentImplicitlyIs(0), format: FormatSpec { fill: None, align: AlignUnknown, @@ -658,7 +660,7 @@ mod tests { fn format_flags() { same("{:-}", &[NextArgument(Argument { - position: ArgumentIs(0), + position: ArgumentImplicitlyIs(0), format: FormatSpec { fill: None, align: AlignUnknown, @@ -670,7 +672,7 @@ mod tests { })]); same("{:+#}", &[NextArgument(Argument { - position: ArgumentIs(0), + position: ArgumentImplicitlyIs(0), format: FormatSpec { fill: None, align: AlignUnknown, diff --git a/src/librustc/traits/on_unimplemented.rs b/src/librustc/traits/on_unimplemented.rs index 94f6efcad4ad..16e200d56f9f 100644 --- a/src/librustc/traits/on_unimplemented.rs +++ b/src/librustc/traits/on_unimplemented.rs @@ -254,7 +254,7 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedFormatString { } }, // `{:1}` and `{}` are not to be used - Position::ArgumentIs(_) => { + Position::ArgumentIs(_) | Position::ArgumentImplicitlyIs(_) => { span_err!(tcx.sess, span, E0231, "only named substitution \ parameters are allowed"); diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index 63c533df198d..ad5bd39a4534 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -110,6 +110,8 @@ struct Context<'a, 'b: 'a> { /// still existed in this phase of processing. /// Used only for `all_pieces_simple` tracking in `trans_piece`. curarg: usize, + /// Keep track of invalid references to positional arguments + invalid_refs: Vec, } /// Parses the arguments from the given list of tokens, returning None @@ -226,7 +228,7 @@ impl<'a, 'b> Context<'a, 'b> { // argument second, if it's an implicit positional parameter // it's written second, so it should come after width/precision. let pos = match arg.position { - parse::ArgumentIs(i) => Exact(i), + parse::ArgumentIs(i) | parse::ArgumentImplicitlyIs(i) => Exact(i), parse::ArgumentNamed(s) => Named(s.to_string()), }; @@ -251,23 +253,54 @@ impl<'a, 'b> Context<'a, 'b> { fn describe_num_args(&self) -> String { match self.args.len() { - 0 => "no arguments given".to_string(), + 0 => "no arguments were given".to_string(), 1 => "there is 1 argument".to_string(), x => format!("there are {} arguments", x), } } + /// Handle invalid references to positional arguments. Output different + /// errors for the case where all arguments are positional and for when + /// there are named arguments or numbered positional arguments in the + /// format string. + fn report_invalid_references(&self, numbered_position_args: bool) { + let mut e; + let mut refs: Vec = self.invalid_refs + .iter() + .map(|r| r.to_string()) + .collect(); + + if self.names.is_empty() && !numbered_position_args { + e = self.ecx.mut_span_err(self.fmtsp, + &format!("{} positional argument{} in format string, but {}", + self.pieces.len(), + if self.pieces.len() > 1 { "s" } else { "" }, + self.describe_num_args())); + } else { + let arg_list = match refs.len() { + 1 => format!("argument {}", refs.pop().unwrap()), + _ => format!("arguments {head} and {tail}", + tail=refs.pop().unwrap(), + head=refs.join(", ")) + }; + + e = self.ecx.mut_span_err(self.fmtsp, + &format!("invalid reference to positional {} ({})", + arg_list, + self.describe_num_args())); + e.note("positional arguments are zero-based"); + }; + + e.emit(); + } + /// Actually verifies and tracks a given format placeholder /// (a.k.a. argument). fn verify_arg_type(&mut self, arg: Position, ty: ArgumentType) { match arg { Exact(arg) => { if self.args.len() <= arg { - let msg = format!("invalid reference to argument `{}` ({})", - arg, - self.describe_num_args()); - - self.ecx.span_err(self.fmtsp, &msg[..]); + self.invalid_refs.push(arg); return; } match ty { @@ -403,7 +436,8 @@ impl<'a, 'b> Context<'a, 'b> { } }; match arg.position { - parse::ArgumentIs(i) => { + parse::ArgumentIs(i) + | parse::ArgumentImplicitlyIs(i) => { // Map to index in final generated argument array // in case of multiple types specified let arg_idx = match arg_index_consumed.get_mut(i) { @@ -691,6 +725,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt, all_pieces_simple: true, macsp, fmtsp: fmt.span, + invalid_refs: Vec::new(), }; let fmt_str = &*fmt.node.0.as_str(); @@ -711,6 +746,18 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt, } } + let numbered_position_args = pieces.iter().any(|arg: &parse::Piece| { + match *arg { + parse::String(_) => false, + parse::NextArgument(arg) => { + match arg.position { + parse::Position::ArgumentIs(_) => true, + _ => false, + } + } + } + }); + cx.build_index_map(); let mut arg_index_consumed = vec![0usize; cx.arg_index_map.len()]; @@ -736,6 +783,10 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt, cx.str_pieces.push(s); } + if cx.invalid_refs.len() >= 1 { + cx.report_invalid_references(numbered_position_args); + } + // Make sure that all arguments were used and all arguments have types. let num_pos_args = cx.args.len() - cx.names.len(); let mut errs = vec![]; diff --git a/src/test/compile-fail/ifmt-bad-arg.rs b/src/test/compile-fail/ifmt-bad-arg.rs index a23b4b077410..afe9bc152a36 100644 --- a/src/test/compile-fail/ifmt-bad-arg.rs +++ b/src/test/compile-fail/ifmt-bad-arg.rs @@ -11,36 +11,44 @@ fn main() { // bad arguments to the format! call - format!("{}"); //~ ERROR: invalid reference to argument + // bad number of arguments, see #44954 (originally #15780) - format!("{1}", 1); //~ ERROR: invalid reference to argument `1` - //~^ ERROR: argument never used - format!("{foo}"); //~ ERROR: no argument named `foo` + format!("{}"); + //~^ ERROR: 1 positional argument in format string, but no arguments were given - format!("", 1, 2); //~ ERROR: multiple unused formatting arguments - format!("{}", 1, 2); //~ ERROR: argument never used - format!("{1}", 1, 2); //~ ERROR: argument never used - format!("{}", 1, foo=2); //~ ERROR: named argument never used - format!("{foo}", 1, foo=2); //~ ERROR: argument never used - format!("", foo=2); //~ ERROR: named argument never used + format!("{1}", 1); + //~^ ERROR: invalid reference to positional argument 1 (there is 1 argument) + //~^^ ERROR: argument never used - format!("{foo}", foo=1, foo=2); //~ ERROR: duplicate argument - format!("", foo=1, 2); //~ ERROR: positional arguments cannot follow - - // bad number of arguments, see #15780 - - format!("{0}"); - //~^ ERROR invalid reference to argument `0` (no arguments given) + format!("{} {}"); + //~^ ERROR: 2 positional arguments in format string, but no arguments were given format!("{0} {1}", 1); - //~^ ERROR invalid reference to argument `1` (there is 1 argument) + //~^ ERROR: invalid reference to positional argument 1 (there is 1 argument) format!("{0} {1} {2}", 1, 2); - //~^ ERROR invalid reference to argument `2` (there are 2 arguments) - - format!("{0} {1}"); - //~^ ERROR invalid reference to argument `0` (no arguments given) - //~^^ ERROR invalid reference to argument `1` (no arguments given) + //~^ ERROR: invalid reference to positional argument 2 (there are 2 arguments) + + format!("{} {value} {} {}", 1, value=2); + //~^ ERROR: invalid reference to positional argument 2 (there are 2 arguments) + format!("{name} {value} {} {} {} {} {} {}", 0, name=1, value=2); + //~^ ERROR: invalid reference to positional arguments 3, 4 and 5 (there are 3 arguments) + + format!("{} {foo} {} {bar} {}", 1, 2, 3); + //~^ ERROR: there is no argument named `foo` + //~^^ ERROR: there is no argument named `bar` + + format!("{foo}"); //~ ERROR: no argument named `foo` + format!("", 1, 2); //~ ERROR: multiple unused formatting arguments + format!("{}", 1, 2); //~ ERROR: argument never used + format!("{1}", 1, 2); //~ ERROR: argument never used + format!("{}", 1, foo=2); //~ ERROR: named argument never used + format!("{foo}", 1, foo=2); //~ ERROR: argument never used + format!("", foo=2); //~ ERROR: named argument never used + format!("{} {}", 1, 2, foo=1, bar=2); //~ ERROR: multiple unused formatting arguments + + format!("{foo}", foo=1, foo=2); //~ ERROR: duplicate argument + format!("", foo=1, 2); //~ ERROR: positional arguments cannot follow // bad named arguments, #35082 diff --git a/src/test/ui/cross-crate-macro-backtrace/main.stderr b/src/test/ui/cross-crate-macro-backtrace/main.stderr index 84db85ac092d..4dad6d60b402 100644 --- a/src/test/ui/cross-crate-macro-backtrace/main.stderr +++ b/src/test/ui/cross-crate-macro-backtrace/main.stderr @@ -1,4 +1,4 @@ -error: invalid reference to argument `0` (no arguments given) +error: 1 positional argument in format string, but no arguments were given --> $DIR/main.rs:16:5 | 16 | myprintln!("{}"); //~ ERROR in this macro diff --git a/src/test/ui/macros/macro-backtrace-println.stderr b/src/test/ui/macros/macro-backtrace-println.stderr index f21253bb67fb..3e782294484e 100644 --- a/src/test/ui/macros/macro-backtrace-println.stderr +++ b/src/test/ui/macros/macro-backtrace-println.stderr @@ -1,4 +1,4 @@ -error: invalid reference to argument `0` (no arguments given) +error: 1 positional argument in format string, but no arguments were given --> $DIR/macro-backtrace-println.rs:24:30 | 24 | ($fmt:expr) => (myprint!(concat!($fmt, "/n")));