Skip to content

Commit

Permalink
Rollup merge of rust-lang#94030 - ChayimFriedman2:issue-94010, r=petr…
Browse files Browse the repository at this point in the history 
…ochenkov

Correctly mark the span of captured arguments in `format_args!()`

It should not include the braces, or misspelling suggestions will be wrong.

Fixes rust-lang#94010.
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored Feb 16, 2022
2 parents 6156482 + 91adb6c commit a8c7dd4
Show file tree
Hide file tree
Showing 15 changed files with 105 additions and 74 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_builtin_macros/src/asm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -700,11 +700,11 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, args: AsmArgs) -> Option<ast::Inl
Some(idx)
}
}
parse::ArgumentNamed(name) => match args.named_args.get(&name) {
parse::ArgumentNamed(name, span) => match args.named_args.get(&name) {
Some(&idx) => Some(idx),
None => {
let msg = format!("there is no argument named `{}`", name);
ecx.struct_span_err(span, &msg).emit();
ecx.struct_span_err(template_span.from_inner(span), &msg).emit();
None
}
},
Expand Down
26 changes: 13 additions & 13 deletions compiler/rustc_builtin_macros/src/format.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc_errors::{pluralize, Applicability, DiagnosticBuilder};
use rustc_expand::base::{self, *};
use rustc_parse_format as parse;
use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::{MultiSpan, Span};
use rustc_span::{InnerSpan, MultiSpan, Span};
use smallvec::SmallVec;

use std::borrow::Cow;
Expand All @@ -26,7 +26,7 @@ enum ArgumentType {
enum Position {
Exact(usize),
Capture(usize),
Named(Symbol),
Named(Symbol, InnerSpan),
}

struct Context<'a, 'b> {
Expand Down Expand Up @@ -247,13 +247,13 @@ impl<'a, 'b> Context<'a, 'b> {
match *p {
parse::String(_) => {}
parse::NextArgument(ref mut arg) => {
if let parse::ArgumentNamed(s) = arg.position {
if let parse::ArgumentNamed(s, _) = arg.position {
arg.position = parse::ArgumentIs(lookup(s));
}
if let parse::CountIsName(s) = arg.format.width {
if let parse::CountIsName(s, _) = arg.format.width {
arg.format.width = parse::CountIsParam(lookup(s));
}
if let parse::CountIsName(s) = arg.format.precision {
if let parse::CountIsName(s, _) = arg.format.precision {
arg.format.precision = parse::CountIsParam(lookup(s));
}
}
Expand All @@ -276,7 +276,7 @@ impl<'a, 'b> Context<'a, 'b> {
// it's written second, so it should come after width/precision.
let pos = match arg.position {
parse::ArgumentIs(i) | parse::ArgumentImplicitlyIs(i) => Exact(i),
parse::ArgumentNamed(s) => Named(s),
parse::ArgumentNamed(s, span) => Named(s, span),
};

let ty = Placeholder(match arg.format.ty {
Expand Down Expand Up @@ -346,8 +346,8 @@ impl<'a, 'b> Context<'a, 'b> {
parse::CountIsParam(i) => {
self.verify_arg_type(Exact(i), Count);
}
parse::CountIsName(s) => {
self.verify_arg_type(Named(s), Count);
parse::CountIsName(s, span) => {
self.verify_arg_type(Named(s, span), Count);
}
}
}
Expand Down Expand Up @@ -533,7 +533,7 @@ impl<'a, 'b> Context<'a, 'b> {
}
}

Named(name) => {
Named(name, span) => {
match self.names.get(&name) {
Some(&idx) => {
// Treat as positional arg.
Expand All @@ -548,7 +548,7 @@ impl<'a, 'b> Context<'a, 'b> {
self.arg_types.push(Vec::new());
self.arg_unique_types.push(Vec::new());
let span = if self.is_literal {
*self.arg_spans.get(self.curpiece).unwrap_or(&self.fmtsp)
self.fmtsp.from_inner(span)
} else {
self.fmtsp
};
Expand All @@ -559,7 +559,7 @@ impl<'a, 'b> Context<'a, 'b> {
} else {
let msg = format!("there is no argument named `{}`", name);
let sp = if self.is_literal {
*self.arg_spans.get(self.curpiece).unwrap_or(&self.fmtsp)
self.fmtsp.from_inner(span)
} else {
self.fmtsp
};
Expand Down Expand Up @@ -629,7 +629,7 @@ impl<'a, 'b> Context<'a, 'b> {
}
parse::CountImplied => count(sym::Implied, None),
// should never be the case, names are already resolved
parse::CountIsName(_) => panic!("should never happen"),
parse::CountIsName(..) => panic!("should never happen"),
}
}

Expand Down Expand Up @@ -676,7 +676,7 @@ impl<'a, 'b> Context<'a, 'b> {

// should never be the case, because names are already
// resolved.
parse::ArgumentNamed(_) => panic!("should never happen"),
parse::ArgumentNamed(..) => panic!("should never happen"),
}
};

Expand Down
16 changes: 10 additions & 6 deletions compiler/rustc_parse_format/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ pub enum Position {
/// The argument is located at a specific index given in the format
ArgumentIs(usize),
/// The argument has a name.
ArgumentNamed(Symbol),
ArgumentNamed(Symbol, InnerSpan),
}

impl Position {
Expand Down Expand Up @@ -147,7 +147,7 @@ pub enum Count {
/// The count is specified explicitly.
CountIs(usize),
/// The count is specified by the argument with the given name.
CountIsName(Symbol),
CountIsName(Symbol, InnerSpan),
/// The count is specified by the argument at the given index.
CountIsParam(usize),
/// The count is implied and cannot be explicitly specified.
Expand Down Expand Up @@ -494,8 +494,11 @@ impl<'a> Parser<'a> {
Some(ArgumentIs(i))
} else {
match self.cur.peek() {
Some(&(_, c)) if rustc_lexer::is_id_start(c) => {
Some(ArgumentNamed(Symbol::intern(self.word())))
Some(&(start, c)) if rustc_lexer::is_id_start(c) => {
let word = self.word();
let end = start + word.len();
let span = self.to_span_index(start).to(self.to_span_index(end));
Some(ArgumentNamed(Symbol::intern(word), span))
}

// This is an `ArgumentNext`.
Expand Down Expand Up @@ -662,8 +665,9 @@ impl<'a> Parser<'a> {
if word.is_empty() {
self.cur = tmp;
(CountImplied, None)
} else if self.consume('$') {
(CountIsName(Symbol::intern(word)), None)
} else if let Some(end) = self.consume_pos('$') {
let span = self.to_span_index(start + 1).to(self.to_span_index(end));
(CountIsName(Symbol::intern(word), span), None)
} else {
self.cur = tmp;
(CountImplied, None)
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_parse_format/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,8 @@ fn format_counts() {
fill: None,
align: AlignUnknown,
flags: 0,
precision: CountIsName(Symbol::intern("b")),
width: CountIsName(Symbol::intern("a")),
precision: CountIsName(Symbol::intern("b"), InnerSpan::new(6, 7)),
width: CountIsName(Symbol::intern("a"), InnerSpan::new(4, 4)),
precision_span: None,
width_span: None,
ty: "?",
Expand Down
16 changes: 8 additions & 8 deletions compiler/rustc_trait_selection/src/traits/on_unimplemented.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,23 +309,23 @@ impl<'tcx> OnUnimplementedFormatString {
Piece::String(_) => (), // Normal string, no need to check it
Piece::NextArgument(a) => match a.position {
// `{Self}` is allowed
Position::ArgumentNamed(s) if s == kw::SelfUpper => (),
Position::ArgumentNamed(s, _) if s == kw::SelfUpper => (),
// `{ThisTraitsName}` is allowed
Position::ArgumentNamed(s) if s == name => (),
Position::ArgumentNamed(s, _) if s == name => (),
// `{from_method}` is allowed
Position::ArgumentNamed(s) if s == sym::from_method => (),
Position::ArgumentNamed(s, _) if s == sym::from_method => (),
// `{from_desugaring}` is allowed
Position::ArgumentNamed(s) if s == sym::from_desugaring => (),
Position::ArgumentNamed(s, _) if s == sym::from_desugaring => (),
// `{ItemContext}` is allowed
Position::ArgumentNamed(s) if s == sym::ItemContext => (),
Position::ArgumentNamed(s, _) if s == sym::ItemContext => (),
// `{integral}` and `{integer}` and `{float}` are allowed
Position::ArgumentNamed(s)
Position::ArgumentNamed(s, _)
if s == sym::integral || s == sym::integer_ || s == sym::float =>
{
()
}
// So is `{A}` if A is a type parameter
Position::ArgumentNamed(s) => {
Position::ArgumentNamed(s, _) => {
match generics.params.iter().find(|param| param.name == s) {
Some(_) => (),
None => {
Expand Down Expand Up @@ -392,7 +392,7 @@ impl<'tcx> OnUnimplementedFormatString {
.map(|p| match p {
Piece::String(s) => s,
Piece::NextArgument(a) => match a.position {
Position::ArgumentNamed(s) => match generic_map.get(&s) {
Position::ArgumentNamed(s, _) => match generic_map.get(&s) {
Some(val) => val,
None if s == name => &trait_str,
None => {
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/asm/bad-template.aarch64_mirunsafeck.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ LL | asm!("{1}", in(reg) foo);
= help: if this argument is intentionally unused, consider using it in an asm comment: `"/* {0} */"`

error: there is no argument named `a`
--> $DIR/bad-template.rs:36:15
--> $DIR/bad-template.rs:36:16
|
LL | asm!("{a}");
| ^^^
| ^

error: invalid reference to argument at index 0
--> $DIR/bad-template.rs:38:15
Expand Down Expand Up @@ -123,10 +123,10 @@ LL | global_asm!("{1}", const FOO);
= help: if this argument is intentionally unused, consider using it in an asm comment: `"/* {0} */"`

error: there is no argument named `a`
--> $DIR/bad-template.rs:63:14
--> $DIR/bad-template.rs:63:15
|
LL | global_asm!("{a}");
| ^^^
| ^

error: invalid reference to argument at index 0
--> $DIR/bad-template.rs:65:14
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/asm/bad-template.aarch64_thirunsafeck.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ LL | asm!("{1}", in(reg) foo);
= help: if this argument is intentionally unused, consider using it in an asm comment: `"/* {0} */"`

error: there is no argument named `a`
--> $DIR/bad-template.rs:36:15
--> $DIR/bad-template.rs:36:16
|
LL | asm!("{a}");
| ^^^
| ^

error: invalid reference to argument at index 0
--> $DIR/bad-template.rs:38:15
Expand Down Expand Up @@ -123,10 +123,10 @@ LL | global_asm!("{1}", const FOO);
= help: if this argument is intentionally unused, consider using it in an asm comment: `"/* {0} */"`

error: there is no argument named `a`
--> $DIR/bad-template.rs:63:14
--> $DIR/bad-template.rs:63:15
|
LL | global_asm!("{a}");
| ^^^
| ^

error: invalid reference to argument at index 0
--> $DIR/bad-template.rs:65:14
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/asm/bad-template.x86_64_mirunsafeck.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ LL | asm!("{1}", in(reg) foo);
= help: if this argument is intentionally unused, consider using it in an asm comment: `"/* {0} */"`

error: there is no argument named `a`
--> $DIR/bad-template.rs:36:15
--> $DIR/bad-template.rs:36:16
|
LL | asm!("{a}");
| ^^^
| ^

error: invalid reference to argument at index 0
--> $DIR/bad-template.rs:38:15
Expand Down Expand Up @@ -123,10 +123,10 @@ LL | global_asm!("{1}", const FOO);
= help: if this argument is intentionally unused, consider using it in an asm comment: `"/* {0} */"`

error: there is no argument named `a`
--> $DIR/bad-template.rs:63:14
--> $DIR/bad-template.rs:63:15
|
LL | global_asm!("{a}");
| ^^^
| ^

error: invalid reference to argument at index 0
--> $DIR/bad-template.rs:65:14
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/asm/bad-template.x86_64_thirunsafeck.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ LL | asm!("{1}", in(reg) foo);
= help: if this argument is intentionally unused, consider using it in an asm comment: `"/* {0} */"`

error: there is no argument named `a`
--> $DIR/bad-template.rs:36:15
--> $DIR/bad-template.rs:36:16
|
LL | asm!("{a}");
| ^^^
| ^

error: invalid reference to argument at index 0
--> $DIR/bad-template.rs:38:15
Expand Down Expand Up @@ -123,10 +123,10 @@ LL | global_asm!("{1}", const FOO);
= help: if this argument is intentionally unused, consider using it in an asm comment: `"/* {0} */"`

error: there is no argument named `a`
--> $DIR/bad-template.rs:63:14
--> $DIR/bad-template.rs:63:15
|
LL | global_asm!("{a}");
| ^^^
| ^

error: invalid reference to argument at index 0
--> $DIR/bad-template.rs:65:14
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/fmt/format-args-capture-issue-93378.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ error: invalid reference to positional argument 0 (no arguments were given)
--> $DIR/format-args-capture-issue-93378.rs:9:23
|
LL | println!("{a:.n$} {b:.*}");
| ------- ^^^--^
| | |
| | this precision flag adds an extra required argument at position 0, which is why there are 3 arguments expected
| this parameter corresponds to the precision flag
| - ^^^--^
| | |
| | this precision flag adds an extra required argument at position 0, which is why there are 3 arguments expected
| this parameter corresponds to the precision flag
|
= note: positional arguments are zero-based
= note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html
Expand Down
7 changes: 7 additions & 0 deletions src/test/ui/fmt/format-args-capture-issue-94010.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
fn main() {
const FOO: i32 = 123;
println!("{foo:X}");
//~^ ERROR: cannot find value `foo` in this scope
println!("{:.foo$}", 0);
//~^ ERROR: cannot find value `foo` in this scope
}
20 changes: 20 additions & 0 deletions src/test/ui/fmt/format-args-capture-issue-94010.stderr
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error[E0425]: cannot find value `foo` in this scope
--> $DIR/format-args-capture-issue-94010.rs:3:16
|
LL | const FOO: i32 = 123;
| --------------------- similarly named constant `FOO` defined here
LL | println!("{foo:X}");
| ^^^ help: a constant with a similar name exists (notice the capitalization): `FOO`

error[E0425]: cannot find value `foo` in this scope
--> $DIR/format-args-capture-issue-94010.rs:5:18
|
LL | const FOO: i32 = 123;
| --------------------- similarly named constant `FOO` defined here
...
LL | println!("{:.foo$}", 0);
| ^^^ help: a constant with a similar name exists (notice the capitalization): `FOO`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0425`.
Loading

0 comments on commit a8c7dd4

Please sign in to comment.