Skip to content

Commit a6393f9

Browse files
committed
Expand repr(...) diagnostics
1 parent ecb10bf commit a6393f9

File tree

12 files changed

+765
-207
lines changed

12 files changed

+765
-207
lines changed

compiler/rustc_ast/src/ast.rs

+18
Original file line numberDiff line numberDiff line change
@@ -1552,6 +1552,24 @@ pub enum StrStyle {
15521552
Raw(u16),
15531553
}
15541554

1555+
impl StrStyle {
1556+
/// Actual string data offset from the start of [Lit] span
1557+
pub fn prefix_len(self) -> u32 {
1558+
match self {
1559+
Self::Cooked => 1,
1560+
Self::Raw(n) => u32::from(n) + 2, // r#" is 1 + `#` count long + 1
1561+
}
1562+
}
1563+
1564+
/// Actual string data offset from the end of [Lit] span
1565+
pub fn suffix_len(self) -> u32 {
1566+
match self {
1567+
Self::Cooked => 1,
1568+
Self::Raw(n) => u32::from(n) + 1, // no starting `r` is present, only add 1
1569+
}
1570+
}
1571+
}
1572+
15551573
/// An AST literal.
15561574
#[derive(Clone, Encodable, Decodable, Debug, HashStable_Generic)]
15571575
pub struct Lit {

compiler/rustc_attr/src/builtin.rs

+142-40
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use rustc_macros::HashStable_Generic;
88
use rustc_session::parse::{feature_err, ParseSess};
99
use rustc_session::Session;
1010
use rustc_span::hygiene::Transparency;
11-
use rustc_span::{symbol::sym, symbol::Symbol, Span};
11+
use rustc_span::{symbol::sym, symbol::Symbol, BytePos, Span};
1212
use std::num::NonZeroU32;
1313

1414
pub fn is_builtin_attr(attr: &Attribute) -> bool {
@@ -843,10 +843,10 @@ impl ReprAttr {
843843
"invalid `repr({})` attribute: no arguments expected",
844844
name,
845845
)
846-
.span_suggestion(
846+
.span_suggestion_verbose(
847847
item.path.span.shrink_to_hi().to(item.span.shrink_to_hi()),
848848
"remove additional values",
849-
format!("{}", name),
849+
String::new(),
850850
Applicability::MachineApplicable,
851851
)
852852
.emit();
@@ -862,55 +862,107 @@ impl ReprAttr {
862862
name
863863
);
864864
match value.kind {
865-
ast::LitKind::Int(int, ast::LitIntType::Unsuffixed) => {
866-
err.span_suggestion(
867-
item.span,
865+
ast::LitKind::Int(_int, ast::LitIntType::Unsuffixed) => {
866+
let open_paren_span =
867+
item.path.span.shrink_to_hi().to(value.span.shrink_to_lo());
868+
let close_paren_span =
869+
item.span.shrink_to_hi().to(value.span.shrink_to_hi());
870+
871+
err.multipart_suggestion(
868872
"use parentheses instead",
869-
format!("{}({})", name, int),
873+
vec![
874+
(open_paren_span, "(".to_string()),
875+
(close_paren_span, ")".to_string()),
876+
],
870877
Applicability::MachineApplicable,
871878
);
872879
}
873-
ast::LitKind::Str(s, _) => {
874-
err.span_suggestion(
875-
item.span,
876-
"use parentheses instead",
877-
format!("{}({})", name, s),
878-
Applicability::MachineApplicable,
879-
);
880+
ast::LitKind::Str(str, style) => {
881+
// repr(foo = "123" )
882+
// ^^^ ^
883+
let open_paren_span =
884+
item.path.span.shrink_to_hi().to(value.span.shrink_to_lo());
885+
let close_paren_span =
886+
item.span.shrink_to_hi().to(value.span.shrink_to_hi());
887+
// repr(foo = "123" )
888+
// ^^^^ ^^
889+
// BytePos math is safe because strings can only be surrounded by single
890+
// byte characters (`r`, `#`, and `"` are all one byte)
891+
let open_paren_span = open_paren_span
892+
.with_hi(open_paren_span.hi() + BytePos(style.prefix_len()));
893+
let close_paren_span = close_paren_span
894+
.with_hi(close_paren_span.lo() - BytePos(style.suffix_len()));
895+
896+
if str.is_empty() {
897+
// When encountering repr(foo = "") suggest additional help
898+
err.span_suggestion_verbose(
899+
open_paren_span.to(close_paren_span),
900+
"use parentheses instead and add a value",
901+
"(/* value */)".to_string(),
902+
Applicability::HasPlaceholders,
903+
);
904+
} else {
905+
err.multipart_suggestion(
906+
"use parentheses instead",
907+
vec![
908+
(open_paren_span, "(".to_string()),
909+
(close_paren_span, ")".to_string()),
910+
],
911+
Applicability::MachineApplicable,
912+
);
913+
914+
match str.as_str().parse::<u128>() {
915+
Ok(num) => match is_alignment_valid(num) {
916+
Ok(align) => {
917+
err.emit();
918+
return Some(align);
919+
}
920+
Err(msg) => {
921+
err.span_label(value.span, msg);
922+
}
923+
},
924+
Err(_) => {
925+
err.span_label(value.span, "must be a non-negative number");
926+
}
927+
}
928+
}
929+
}
930+
_ => {
931+
err.span_label(value.span, "must be a non-negative number");
880932
}
881-
_ => {}
882933
}
883934
err.emit();
884935
} else if item.is_word() || matches!(item.meta_item_list(), Some([])) {
885936
struct_span_err!(
886937
diagnostic,
887938
item.span,
888939
E0693,
889-
"invalid `repr({})` attribute: expected a value",
940+
"invalid `repr({})` attribute: expected a non-negative number",
890941
name
891942
)
892-
.span_suggestion(
893-
item.span,
943+
.span_suggestion_verbose(
944+
item.path.span.shrink_to_hi().to(item.span.shrink_to_hi()),
894945
"add a value",
895-
format!("{}(/* value */)", name),
946+
"(/* value */)".to_string(),
896947
Applicability::HasPlaceholders,
897948
)
898949
.emit();
899-
} else if let Some([NestedMetaItem::Literal(value)]) = item.meta_item_list() {
900-
match parse_alignment(&value.kind) {
901-
Ok(literal) => return Some(literal),
902-
Err(message) => {
903-
struct_span_err!(
904-
diagnostic,
905-
item.span,
906-
E0589,
907-
"invalid `repr({})` attribute: {}",
908-
name,
909-
message
910-
)
911-
.emit();
950+
} else if let Some([meta]) = item.meta_item_list() {
951+
match parse_alignment(
952+
struct_span_err!(
953+
diagnostic,
954+
meta.span(),
955+
E0589,
956+
"invalid `repr({})` attribute",
957+
name,
958+
),
959+
meta,
960+
) {
961+
Ok(lit) => return Some(lit),
962+
Err(()) => {
963+
// parse_alignment already emitted an error
912964
}
913-
};
965+
}
914966
} else if let Some([first_meta, .., last_meta]) = item.meta_item_list() {
915967
struct_span_err!(
916968
diagnostic,
@@ -1124,15 +1176,65 @@ fn allow_unstable<'a>(
11241176
})
11251177
}
11261178

1127-
pub fn parse_alignment(node: &ast::LitKind) -> Result<u32, &'static str> {
1128-
if let ast::LitKind::Int(literal, ast::LitIntType::Unsuffixed) = node {
1129-
if literal.is_power_of_two() {
1130-
// rustc_middle::ty::layout::Align restricts align to <= 2^29
1131-
if *literal <= 1 << 29 { Ok(*literal as u32) } else { Err("larger than 2^29") }
1179+
pub fn parse_alignment(
1180+
mut err: rustc_errors::DiagnosticBuilder<'_>,
1181+
node: &ast::NestedMetaItem,
1182+
) -> Result<u32, ()> {
1183+
if let ast::NestedMetaItem::Literal(lit) = node {
1184+
if let ast::LitKind::Int(val, ast::LitIntType::Unsuffixed) = lit.kind {
1185+
match is_alignment_valid(val) {
1186+
Ok(align) => {
1187+
err.cancel();
1188+
return Ok(align);
1189+
}
1190+
Err(msg) => {
1191+
err.span_label(lit.span, msg);
1192+
}
1193+
}
1194+
} else if let ast::LitKind::Str(sym, style) = lit.kind {
1195+
match sym.as_str().parse::<u128>() {
1196+
Ok(num) => match is_alignment_valid(num) {
1197+
Ok(align) => {
1198+
err.multipart_suggestion(
1199+
"remove the quotes",
1200+
vec![
1201+
(
1202+
lit.span.with_hi(lit.span.lo() + BytePos(style.prefix_len())),
1203+
"".to_string(),
1204+
),
1205+
(
1206+
lit.span.with_lo(lit.span.hi() - BytePos(style.suffix_len())),
1207+
"".to_string(),
1208+
),
1209+
],
1210+
Applicability::MachineApplicable,
1211+
);
1212+
err.emit();
1213+
return Ok(align);
1214+
}
1215+
Err(msg) => {
1216+
err.span_label(lit.span, msg);
1217+
}
1218+
},
1219+
Err(_) => {
1220+
err.span_label(lit.span, "must be a non-negative number");
1221+
}
1222+
}
11321223
} else {
1133-
Err("not a power of two")
1224+
err.span_label(lit.span, "not an unsuffixed integer");
11341225
}
11351226
} else {
1136-
Err("not an unsuffixed integer")
1227+
err.span_label(node.span(), "not a non-negative number");
1228+
}
1229+
err.emit();
1230+
Err(())
1231+
}
1232+
1233+
fn is_alignment_valid(n: u128) -> Result<u32, &'static str> {
1234+
if n.is_power_of_two() {
1235+
// rustc_target::abi::Align restricts align to <= 2^29
1236+
if n <= 1 << 29 { Ok(n as u32) } else { Err("larger than 2^29") }
1237+
} else {
1238+
Err("not a power of two")
11371239
}
11381240
}

compiler/rustc_typeck/src/collect.rs

+18-17
Original file line numberDiff line numberDiff line change
@@ -2906,29 +2906,30 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
29062906
} else if tcx.sess.check_name(attr, sym::repr) {
29072907
codegen_fn_attrs.alignment = match attr.meta_item_list() {
29082908
Some(items) => match items.as_slice() {
2909-
[item] => match item.name_value_literal() {
2910-
Some((sym::align, literal)) => {
2911-
let alignment = rustc_attr::parse_alignment(&literal.kind);
2912-
2913-
match alignment {
2914-
Ok(align) => Some(align),
2915-
Err(msg) => {
2909+
[item] if item.has_name(sym::align) => {
2910+
match item.meta_item_list() {
2911+
Some([meta]) => {
2912+
let alignment = rustc_attr::parse_alignment(
29162913
struct_span_err!(
29172914
tcx.sess.diagnostic(),
2918-
attr.span,
2915+
meta.span(),
29192916
E0589,
2920-
"invalid `repr(align)` attribute: {}",
2921-
msg
2922-
)
2923-
.emit();
2924-
2925-
None
2917+
"invalid `repr(align)` attribute",
2918+
),
2919+
&meta,
2920+
);
2921+
2922+
match alignment {
2923+
Ok(align) => Some(align),
2924+
Err(()) => {
2925+
// Error already emitted by parse_alignment
2926+
None
2927+
}
29262928
}
29272929
}
2930+
_ => None,
29282931
}
2929-
_ => None,
2930-
},
2931-
[] => None,
2932+
}
29322933
_ => None,
29332934
},
29342935
None => None,

src/test/ui/repr/basic.rs

+33-12
Original file line numberDiff line numberDiff line change
@@ -33,37 +33,58 @@ struct S6(u32);
3333
//~| ERROR invalid `repr(no_niche)` attribute: no arguments expected
3434
struct S7(u32);
3535

36-
#[repr(align())] //~ ERROR invalid `repr(align)` attribute: expected a value
37-
//~| ERROR invalid `repr(align)` attribute: expected a value
36+
37+
#[repr(align)] //~ ERROR invalid `repr(align)` attribute: expected a non-negative number
38+
//~| ERROR invalid `repr(align)` attribute: expected a non-negative number
3839
struct S8(u32);
3940

40-
#[repr(align(1, 2, 3))] //~ ERROR invalid `repr(align)` attribute: expected only one value
41-
//~| ERROR invalid `repr(align)` attribute: expected only one value
41+
#[repr(align())] //~ ERROR invalid `repr(align)` attribute: expected a non-negative number
42+
//~| ERROR invalid `repr(align)` attribute: expected a non-negative number
4243
struct S9(u32);
4344

44-
#[repr(packed())] //~ ERROR invalid `repr(packed)` attribute: expected a value
45-
//~| ERROR invalid `repr(packed)` attribute: expected a value
45+
#[repr(align(foo()))] //~ ERROR invalid `repr(align)` attribute
46+
//~| ERROR invalid `repr(align)` attribute
4647
struct S10(u32);
4748

49+
#[repr(align = "1")] //~ ERROR incorrect `repr(align)` attribute
50+
//~| ERROR incorrect `repr(align)` attribute
51+
struct S11(u32);
52+
53+
#[repr(align = "")] //~ ERROR incorrect `repr(align)` attribute
54+
//~| ERROR incorrect `repr(align)` attribute
55+
struct S12(u32);
56+
57+
#[repr(align = true)] //~ ERROR incorrect `repr(align)` attribute
58+
//~| ERROR incorrect `repr(align)` attribute
59+
struct S13(u32);
60+
61+
#[repr(align(1, 2, 3))] //~ ERROR invalid `repr(align)` attribute: expected only one value
62+
//~| ERROR invalid `repr(align)` attribute: expected only one value
63+
struct S14(u32);
64+
65+
#[repr(packed())] //~ ERROR invalid `repr(packed)` attribute: expected a non-negative number
66+
//~| ERROR invalid `repr(packed)` attribute: expected a non-negative number
67+
struct S15(u32);
68+
4869
#[repr(packed(1, 2, 3))] //~ ERROR invalid `repr(packed)` attribute: expected only one value
4970
//~| ERROR invalid `repr(packed)` attribute: expected only one value
50-
struct S11(u32);
71+
struct S16(u32);
5172

5273
#[repr(i8())] //~ ERROR invalid `repr(i8)` attribute: no arguments expected
5374
//~| ERROR invalid `repr(i8)` attribute: no arguments expected
54-
enum S12 { A, B }
75+
enum S17 { A, B }
5576

5677
#[repr(i8(1, 2, 3))] //~ ERROR invalid `repr(i8)` attribute: no arguments expected
5778
//~| ERROR invalid `repr(i8)` attribute: no arguments expected
58-
enum S13 { A, B }
79+
enum S18 { A, B }
5980

6081
#[repr] //~ ERROR malformed `repr` attribute input
61-
struct S14(u32);
82+
struct S19(u32);
6283

6384
#[repr(123)] //~ ERROR meta item in `repr` must be an identifier
64-
struct S15(u32);
85+
struct S20(u32);
6586

6687
#[repr("foo")] //~ ERROR meta item in `repr` must be an identifier
67-
struct S16(u32);
88+
struct S21(u32);
6889

6990
fn main() {}

0 commit comments

Comments
 (0)