Skip to content

Commit 2ab6205

Browse files
estebankestebank
committed
Modify diagnostic to be more accurate
1 parent 81f402f commit 2ab6205

3 files changed

Lines changed: 136 additions & 21 deletions

File tree

‎src/libsyntax_ext/concat.rs‎

Lines changed: 55 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ pub fn expand_syntax_ext(
2828
};
2929
let mut string_accumulator = String::new();
3030
let mut string_pos = vec![];
31+
let mut int_pos = vec![];
32+
let mut bool_pos = vec![];
3133
let mut b_accumulator: Vec<u8> = vec![];
3234
let mut b_pos: Vec<Span> = vec![];
3335
// We don't support mixing things with byte str literals, but do a best effort to fill in a
@@ -42,6 +44,8 @@ pub fn expand_syntax_ext(
4244
| ast::LitKind::FloatUnsuffixed(ref s) => {
4345
string_accumulator.push_str(&s.as_str());
4446
string_pos.push(e.span);
47+
// If we ever allow `concat!("", b"")`, we should probably add a warn by default
48+
// lint to this code.
4549
unified_accumulator.extend(s.to_string().into_bytes());
4650
}
4751
ast::LitKind::Char(c) => {
@@ -53,13 +57,19 @@ pub fn expand_syntax_ext(
5357
| ast::LitKind::Int(i, ast::LitIntType::Signed(_))
5458
| ast::LitKind::Int(i, ast::LitIntType::Unsuffixed) => {
5559
string_accumulator.push_str(&i.to_string());
56-
string_pos.push(e.span);
60+
int_pos.push(e.span);
61+
// If we ever allow `concat!()` mixing byte literals with integers, we need to
62+
// define the appropriate behavior for it. Consistently considering them as
63+
// "machine width" would be bug-prone. Taking the smallest possible size for the
64+
// literal is probably what people _that don't think about it_ would expect, but
65+
// would be inconsistent. Another option is only to accept the literals if they
66+
// would fit in a `u8`.
5767
unified_accumulator.extend(i.to_bytes().iter());
5868
}
5969
ast::LitKind::Bool(b) => {
6070
string_accumulator.push_str(&b.to_string());
61-
string_pos.push(e.span);
62-
unified_accumulator.push(b as u8);
71+
bool_pos.push(e.span);
72+
// would `concat!(true, b"asdf")` ever make sense?
6373
}
6474
ast::LitKind::Byte(byte) => {
6575
b_accumulator.push(byte);
@@ -71,8 +81,10 @@ pub fn expand_syntax_ext(
7181
b_pos.push(e.span);
7282
unified_accumulator.extend(b_str.iter());
7383
}
74-
},
84+
}
7585
_ => {
86+
// Consider the possibility of allowing `concat!(b"asdf", [1, 2, 3, 4])`, given
87+
// that every single element of the array is a valid `u8`.
7688
missing_literal.push(e.span);
7789
}
7890
}
@@ -84,26 +96,54 @@ pub fn expand_syntax_ext(
8496
}
8597
let sp = sp.apply_mark(cx.current_expansion.mark);
8698
// Do not allow mixing "" and b"", but return the joint b"" to avoid further errors
87-
if string_accumulator.len() > 0 && b_accumulator.len() > 0 {
99+
if b_pos.len() > 0 && (string_pos.len() > 0 || int_pos.len() > 0 || bool_pos.len() > 0) {
100+
let mut mixings = vec![];
101+
if string_pos.len() > 0 {
102+
mixings.push("string");
103+
}
104+
if int_pos.len() > 0 {
105+
mixings.push("numeric");
106+
}
107+
if bool_pos.len() > 0 {
108+
mixings.push("boolean");
109+
}
88110
let mut err = cx.struct_span_err(
89111
b_pos.clone(),
90-
"cannot concatenate a byte string literal with string literals",
112+
"cannot concatenate a byte string literal with other literals",
91113
);
114+
if mixings.len() > 0 && (int_pos.len() > 0 || bool_pos.len() > 0) {
115+
let msg = if mixings.len() >= 2 {
116+
format!(
117+
"{} or {}",
118+
mixings[0..mixings.len() - 1].join(", "),
119+
mixings.last().unwrap(),
120+
)
121+
} else {
122+
mixings[0].to_string()
123+
};
124+
err.note(&format!("we don't support mixing {} literals and byte strings", msg));
125+
}
126+
if string_pos.len() > 0 && int_pos.len() == 0 && bool_pos.len() == 0 {
127+
err.multipart_suggestion(
128+
"we don't support mixing string and byte string literals, use only byte strings",
129+
string_pos
130+
.iter()
131+
.map(|pos| (pos.shrink_to_lo(), "b".to_string()))
132+
.collect(),
133+
);
134+
}
92135
for pos in &b_pos {
93136
err.span_label(*pos, "byte string literal");
94137
}
95138
for pos in &string_pos {
96139
err.span_label(*pos, "string literal");
97-
98140
}
99-
err.help("do not mix byte string literals and string literals");
100-
err.multipart_suggestion(
101-
"you can use byte string literals",
102-
string_pos
103-
.iter()
104-
.map(|pos| (pos.shrink_to_lo(), "b".to_string()))
105-
.collect(),
106-
);
141+
for pos in &int_pos {
142+
err.span_label(*pos, "numeric literal");
143+
}
144+
for pos in &bool_pos {
145+
err.span_label(*pos, "boolean literal");
146+
}
107147
err.emit();
108148
base::MacEager::expr(cx.expr_byte_str(sp, unified_accumulator))
109149
} else if b_accumulator.len() > 0 {

‎src/test/ui/concat.rs‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,18 @@ fn main() {
1717
//~^ ERROR: expected a literal
1818
concat!(b'a', b"bc", b"def");
1919
concat!("abc", b"def", 'g', "hi", b"jkl");
20-
//~^ ERROR cannot concatenate a byte string literal with string literals
20+
//~^ ERROR cannot concatenate a byte string literal with other literals
2121
// `concat!()` cannot mix "" and b"" literals (it might allow it in the future)
22+
concat!(1, b"def");
23+
//~^ ERROR cannot concatenate a byte string literal with other literals
24+
concat!(true, b"def");
25+
//~^ ERROR cannot concatenate a byte string literal with other literals
26+
concat!(1, true, b"def");
27+
//~^ ERROR cannot concatenate a byte string literal with other literals
28+
concat!(1, true, "abc", b"def");
29+
//~^ ERROR cannot concatenate a byte string literal with other literals
30+
concat!(true, "abc", b"def");
31+
//~^ ERROR cannot concatenate a byte string literal with other literals
32+
concat!(1, "abc", b"def");
33+
//~^ ERROR cannot concatenate a byte string literal with other literals
2234
}

‎src/test/ui/concat.stderr‎

Lines changed: 68 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ LL | concat!(foo());
1414
|
1515
= note: only literals (like `"foo"`, `42` and `3.14`) can be passed to `concat!()`
1616

17-
error: cannot concatenate a byte string literal with string literals
17+
error: cannot concatenate a byte string literal with other literals
1818
--> $DIR/concat.rs:19:20
1919
|
2020
LL | concat!("abc", b"def", 'g', "hi", b"jkl");
@@ -24,12 +24,75 @@ LL | concat!("abc", b"def", 'g', "hi", b"jkl");
2424
| | | string literal
2525
| | byte string literal
2626
| string literal
27-
|
28-
= help: do not mix byte string literals and string literals
29-
help: you can use byte string literals
27+
help: we don't support mixing string and byte string literals, use only byte strings
3028
|
3129
LL | concat!(b"abc", b"def", b'g', b"hi", b"jkl");
3230
| ^ ^ ^
3331

34-
error: aborting due to 3 previous errors
32+
error: cannot concatenate a byte string literal with other literals
33+
--> $DIR/concat.rs:22:16
34+
|
35+
LL | concat!(1, b"def");
36+
| - ^^^^^^ byte string literal
37+
| |
38+
| numeric literal
39+
|
40+
= note: we don't support mixing numeric literals and byte strings
41+
42+
error: cannot concatenate a byte string literal with other literals
43+
--> $DIR/concat.rs:24:19
44+
|
45+
LL | concat!(true, b"def");
46+
| ---- ^^^^^^ byte string literal
47+
| |
48+
| boolean literal
49+
|
50+
= note: we don't support mixing boolean literals and byte strings
51+
52+
error: cannot concatenate a byte string literal with other literals
53+
--> $DIR/concat.rs:26:22
54+
|
55+
LL | concat!(1, true, b"def");
56+
| - ---- ^^^^^^ byte string literal
57+
| | |
58+
| | boolean literal
59+
| numeric literal
60+
|
61+
= note: we don't support mixing numeric or boolean literals and byte strings
62+
63+
error: cannot concatenate a byte string literal with other literals
64+
--> $DIR/concat.rs:28:29
65+
|
66+
LL | concat!(1, true, "abc", b"def");
67+
| - ---- ----- ^^^^^^ byte string literal
68+
| | | |
69+
| | | string literal
70+
| | boolean literal
71+
| numeric literal
72+
|
73+
= note: we don't support mixing string, numeric or boolean literals and byte strings
74+
75+
error: cannot concatenate a byte string literal with other literals
76+
--> $DIR/concat.rs:30:26
77+
|
78+
LL | concat!(true, "abc", b"def");
79+
| ---- ----- ^^^^^^ byte string literal
80+
| | |
81+
| | string literal
82+
| boolean literal
83+
|
84+
= note: we don't support mixing string or boolean literals and byte strings
85+
86+
error: cannot concatenate a byte string literal with other literals
87+
--> $DIR/concat.rs:32:23
88+
|
89+
LL | concat!(1, "abc", b"def");
90+
| - ----- ^^^^^^ byte string literal
91+
| | |
92+
| | string literal
93+
| numeric literal
94+
|
95+
= note: we don't support mixing string or numeric literals and byte strings
96+
97+
error: aborting due to 9 previous errors
3598

0 commit comments

Comments
 (0)