Skip to content

Commit d388428

Browse files
ManishearthManishearth
authored
Rollup merge of #89340 - FabianWolff:issue-89173, r=petrochenkov
Improve error message for `printf`-style format strings Fixes #89173. The following is actually supported today: ```rust fn main() { let num = 5; let width = 20; print!("%*2$x", num, width); } ``` ``` error: multiple unused formatting arguments --> src/main.rs:4:21 | 4 | print!("%*2$x", num, width); | ------- ^^^ ^^^^^ argument never used | || | | || argument never used | |help: format specifiers use curly braces: `{:1$x}` | multiple missing formatting specifiers | = note: printf formatting not supported; see the documentation for `std::fmt` ``` However, as noted in #89173, something like ```rust print!("%0*x", width, num); ``` does not give a helpful suggestion. I think this is partly intended, because there actually _is_ no Rust equivalent to this; you always have to use a positional or named argument to specify the width (instead of just using the "next" argument, as `printf` or even `.*` as a precision specifier in Rust would). Therefore, I have added a note: ``` [...] note: format specifiers use curly braces, and you have to use a positional or named parameter for the width --> t2.rs:4:13 | 4 | print!("%0*x", width, num); | ^^^^ = note: printf formatting not supported; see the documentation for `std::fmt` ``` This is not perfect, but it should at least point the user in the right direction, instead of issuing no explanation at all. cc ```@lcnr```
2 parents 2f67063 + 6490ed3 commit d388428

File tree

6 files changed

+105
-26
lines changed

6 files changed

+105
-26
lines changed

‎compiler/rustc_builtin_macros/src/format.rs

+21-5
Original file line numberDiff line numberDiff line change
@@ -1154,11 +1154,12 @@ pub fn expand_preparsed_format_args(
11541154
// account for `"` and account for raw strings `r#`
11551155
let padding = str_style.map(|i| i + 2).unwrap_or(1);
11561156
for sub in foreign::$kind::iter_subs(fmt_str, padding) {
1157-
let trn = match sub.translate() {
1158-
Some(trn) => trn,
1157+
let (trn, success) = match sub.translate() {
1158+
Ok(trn) => (trn, true),
1159+
Err(Some(msg)) => (msg, false),
11591160

11601161
// If it has no translation, don't call it out specifically.
1161-
None => continue,
1162+
_ => continue,
11621163
};
11631164

11641165
let pos = sub.position();
@@ -1175,9 +1176,24 @@ pub fn expand_preparsed_format_args(
11751176

11761177
if let Some(inner_sp) = pos {
11771178
let sp = fmt_sp.from_inner(inner_sp);
1178-
suggestions.push((sp, trn));
1179+
1180+
if success {
1181+
suggestions.push((sp, trn));
1182+
} else {
1183+
diag.span_note(
1184+
sp,
1185+
&format!("format specifiers use curly braces, and {}", trn),
1186+
);
1187+
}
11791188
} else {
1180-
diag.help(&format!("`{}` should be written as `{}`", sub, trn));
1189+
if success {
1190+
diag.help(&format!("`{}` should be written as `{}`", sub, trn));
1191+
} else {
1192+
diag.note(&format!(
1193+
"`{}` should use curly braces, and {}",
1194+
sub, trn
1195+
));
1196+
}
11811197
}
11821198
}
11831199

‎compiler/rustc_builtin_macros/src/format_foreign.rs

+48-17
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
pub mod printf {
1+
pub(crate) mod printf {
22
use super::strcursor::StrCursor as Cur;
33
use rustc_span::InnerSpan;
44

@@ -36,10 +36,10 @@ pub mod printf {
3636
///
3737
/// This ignores cases where the substitution does not have an exact equivalent, or where
3838
/// the substitution would be unnecessary.
39-
pub fn translate(&self) -> Option<String> {
39+
pub fn translate(&self) -> Result<String, Option<String>> {
4040
match *self {
4141
Substitution::Format(ref fmt) => fmt.translate(),
42-
Substitution::Escape => None,
42+
Substitution::Escape => Err(None),
4343
}
4444
}
4545
}
@@ -68,9 +68,9 @@ pub mod printf {
6868
impl Format<'_> {
6969
/// Translate this directive into an equivalent Rust formatting directive.
7070
///
71-
/// Returns `None` in cases where the `printf` directive does not have an exact Rust
71+
/// Returns `Err` in cases where the `printf` directive does not have an exact Rust
7272
/// equivalent, rather than guessing.
73-
pub fn translate(&self) -> Option<String> {
73+
pub fn translate(&self) -> Result<String, Option<String>> {
7474
use std::fmt::Write;
7575

7676
let (c_alt, c_zero, c_left, c_plus) = {
@@ -84,7 +84,12 @@ pub mod printf {
8484
'0' => c_zero = true,
8585
'-' => c_left = true,
8686
'+' => c_plus = true,
87-
_ => return None,
87+
_ => {
88+
return Err(Some(format!(
89+
"the flag `{}` is unknown or unsupported",
90+
c
91+
)));
92+
}
8893
}
8994
}
9095
(c_alt, c_zero, c_left, c_plus)
@@ -104,7 +109,9 @@ pub mod printf {
104109
let width = match self.width {
105110
Some(Num::Next) => {
106111
// NOTE: Rust doesn't support this.
107-
return None;
112+
return Err(Some(
113+
"you have to use a positional or named parameter for the width".to_string(),
114+
));
108115
}
109116
w @ Some(Num::Arg(_)) => w,
110117
w @ Some(Num::Num(_)) => w,
@@ -125,13 +132,21 @@ pub mod printf {
125132
"p" => (Some(self.type_), false, true),
126133
"g" => (Some("e"), true, false),
127134
"G" => (Some("E"), true, false),
128-
_ => return None,
135+
_ => {
136+
return Err(Some(format!(
137+
"the conversion specifier `{}` is unknown or unsupported",
138+
self.type_
139+
)));
140+
}
129141
};
130142

131143
let (fill, width, precision) = match (is_int, width, precision) {
132144
(true, Some(_), Some(_)) => {
133145
// Rust can't duplicate this insanity.
134-
return None;
146+
return Err(Some(
147+
"width and precision cannot both be specified for integer conversions"
148+
.to_string(),
149+
));
135150
}
136151
(true, None, Some(p)) => (Some("0"), Some(p), None),
137152
(true, w, None) => (fill, w, None),
@@ -169,7 +184,17 @@ pub mod printf {
169184
s.push('{');
170185

171186
if let Some(arg) = self.parameter {
172-
write!(s, "{}", arg.checked_sub(1)?).ok()?;
187+
match write!(
188+
s,
189+
"{}",
190+
match arg.checked_sub(1) {
191+
Some(a) => a,
192+
None => return Err(None),
193+
}
194+
) {
195+
Err(_) => return Err(None),
196+
_ => {}
197+
}
173198
}
174199

175200
if has_options {
@@ -199,12 +224,18 @@ pub mod printf {
199224
}
200225

201226
if let Some(width) = width {
202-
width.translate(&mut s).ok()?;
227+
match width.translate(&mut s) {
228+
Err(_) => return Err(None),
229+
_ => {}
230+
}
203231
}
204232

205233
if let Some(precision) = precision {
206234
s.push('.');
207-
precision.translate(&mut s).ok()?;
235+
match precision.translate(&mut s) {
236+
Err(_) => return Err(None),
237+
_ => {}
238+
}
208239
}
209240

210241
if let Some(type_) = type_ {
@@ -213,7 +244,7 @@ pub mod printf {
213244
}
214245

215246
s.push('}');
216-
Some(s)
247+
Ok(s)
217248
}
218249
}
219250

@@ -623,11 +654,11 @@ pub mod shell {
623654
}
624655
}
625656

626-
pub fn translate(&self) -> Option<String> {
657+
pub fn translate(&self) -> Result<String, Option<String>> {
627658
match *self {
628-
Substitution::Ordinal(n, _) => Some(format!("{{{}}}", n)),
629-
Substitution::Name(n, _) => Some(format!("{{{}}}", n)),
630-
Substitution::Escape(_) => None,
659+
Substitution::Ordinal(n, _) => Ok(format!("{{{}}}", n)),
660+
Substitution::Name(n, _) => Ok(format!("{{{}}}", n)),
661+
Substitution::Escape(_) => Err(None),
631662
}
632663
}
633664
}

‎compiler/rustc_builtin_macros/src/format_foreign/printf/tests.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use super::{iter_subs, parse_next_substitution as pns, Format as F, Num as N, Su
33
macro_rules! assert_eq_pnsat {
44
($lhs:expr, $rhs:expr) => {
55
assert_eq!(
6-
pns($lhs).and_then(|(s, _)| s.translate()),
6+
pns($lhs).and_then(|(s, _)| s.translate().ok()),
77
$rhs.map(<String as From<&str>>::from)
88
)
99
};
@@ -98,7 +98,7 @@ fn test_parse() {
9898
#[test]
9999
fn test_iter() {
100100
let s = "The %d'th word %% is: `%.*s` %!\n";
101-
let subs: Vec<_> = iter_subs(s, 0).map(|sub| sub.translate()).collect();
101+
let subs: Vec<_> = iter_subs(s, 0).map(|sub| sub.translate().ok()).collect();
102102
assert_eq!(
103103
subs.iter().map(|ms| ms.as_ref().map(|s| &s[..])).collect::<Vec<_>>(),
104104
vec![Some("{}"), None, Some("{:.*}"), None]

‎compiler/rustc_builtin_macros/src/format_foreign/shell/tests.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use super::{parse_next_substitution as pns, Substitution as S};
33
macro_rules! assert_eq_pnsat {
44
($lhs:expr, $rhs:expr) => {
55
assert_eq!(
6-
pns($lhs).and_then(|(f, _)| f.translate()),
6+
pns($lhs).and_then(|(f, _)| f.translate().ok()),
77
$rhs.map(<String as From<&str>>::from)
88
)
99
};
@@ -37,7 +37,7 @@ fn test_parse() {
3737
fn test_iter() {
3838
use super::iter_subs;
3939
let s = "The $0'th word $$ is: `$WORD` $!\n";
40-
let subs: Vec<_> = iter_subs(s, 0).map(|sub| sub.translate()).collect();
40+
let subs: Vec<_> = iter_subs(s, 0).map(|sub| sub.translate().ok()).collect();
4141
assert_eq!(
4242
subs.iter().map(|ms| ms.as_ref().map(|s| &s[..])).collect::<Vec<_>>(),
4343
vec![Some("{0}"), None, Some("{WORD}")]

‎src/test/ui/fmt/issue-89173.rs

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Regression test for #89173: Make sure a helpful note is issued for
2+
// printf-style format strings using `*` to specify the width.
3+
4+
fn main() {
5+
let num = 0x0abcde;
6+
let width = 6;
7+
print!("%0*x", width, num);
8+
//~^ ERROR: multiple unused formatting arguments
9+
//~| NOTE: multiple missing formatting specifiers
10+
//~| NOTE: argument never used
11+
//~| NOTE: argument never used
12+
//~| NOTE: format specifiers use curly braces, and you have to use a positional or named parameter for the width
13+
//~| NOTE: printf formatting not supported
14+
}

‎src/test/ui/fmt/issue-89173.stderr

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error: multiple unused formatting arguments
2+
--> $DIR/issue-89173.rs:7:20
3+
|
4+
LL | print!("%0*x", width, num);
5+
| ------ ^^^^^ ^^^ argument never used
6+
| | |
7+
| | argument never used
8+
| multiple missing formatting specifiers
9+
|
10+
note: format specifiers use curly braces, and you have to use a positional or named parameter for the width
11+
--> $DIR/issue-89173.rs:7:13
12+
|
13+
LL | print!("%0*x", width, num);
14+
| ^^^^
15+
= note: printf formatting not supported; see the documentation for `std::fmt`
16+
17+
error: aborting due to previous error
18+

0 commit comments

Comments
 (0)