Skip to content

Commit 8c39ac9

Browse files
borsbors
committed
Auto merge of #127575 - chenyukang:yukang-fix-struct-fields-ice, r=compiler-errors
Avoid "no field" error and ICE on recovered ADT variant Fixes #126744 Fixes #126344, a more general fix compared with #127426 r? `@oli-obk` From `@compiler-errors` 's comment #127502 (comment) Seems most of the ADTs don't have taint, so maybe it's not proper to change `TyCtxt::type_of` query.
2 parents e1f45a1 + 07e6dd9 commit 8c39ac9

File tree

10 files changed

+131
-32
lines changed

10 files changed

+131
-32
lines changed

‎compiler/rustc_hir_analysis/src/collect.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -1125,7 +1125,10 @@ fn lower_variant(
11251125
vis: tcx.visibility(f.def_id),
11261126
})
11271127
.collect();
1128-
let recovered = matches!(def, hir::VariantData::Struct { recovered: Recovered::Yes(_), .. });
1128+
let recovered = match def {
1129+
hir::VariantData::Struct { recovered: Recovered::Yes(guar), .. } => Some(*guar),
1130+
_ => None,
1131+
};
11291132
ty::VariantDef::new(
11301133
ident.name,
11311134
variant_did.map(LocalDefId::to_def_id),

‎compiler/rustc_hir_typeck/src/expr.rs

+7-4
Original file line numberDiff line numberDiff line change
@@ -2177,10 +2177,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
21772177
skip_fields: &[hir::ExprField<'_>],
21782178
kind_name: &str,
21792179
) -> ErrorGuaranteed {
2180-
if variant.is_recovered() {
2181-
let guar =
2182-
self.dcx().span_delayed_bug(expr.span, "parser recovered but no error was emitted");
2183-
self.set_tainted_by_errors(guar);
2180+
// we don't care to report errors for a struct if the struct itself is tainted
2181+
if let Err(guar) = variant.has_errors() {
21842182
return guar;
21852183
}
21862184
let mut err = self.err_ctxt().type_error_struct_with_diag(
@@ -2345,6 +2343,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
23452343
let mut last_ty = None;
23462344
let mut nested_fields = Vec::new();
23472345
let mut index = None;
2346+
2347+
// we don't care to report errors for a struct if the struct itself is tainted
2348+
if let Err(guar) = adt_def.non_enum_variant().has_errors() {
2349+
return Ty::new_error(self.tcx(), guar);
2350+
}
23482351
while let Some(idx) = self.tcx.find_field((adt_def.did(), ident)) {
23492352
let &mut first_idx = index.get_or_insert(idx);
23502353
let field = &adt_def.non_enum_variant().fields[idx];

‎compiler/rustc_hir_typeck/src/pat.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -1531,9 +1531,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
15311531
.filter(|(_, ident)| !used_fields.contains_key(ident))
15321532
.collect::<Vec<_>>();
15331533

1534-
let inexistent_fields_err = if !(inexistent_fields.is_empty() || variant.is_recovered())
1534+
let inexistent_fields_err = if !inexistent_fields.is_empty()
15351535
&& !inexistent_fields.iter().any(|field| field.ident.name == kw::Underscore)
15361536
{
1537+
// we don't care to report errors for a struct if the struct itself is tainted
1538+
variant.has_errors()?;
15371539
Some(self.error_inexistent_fields(
15381540
adt.variant_descr(),
15391541
&inexistent_fields,
@@ -1812,6 +1814,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
18121814
return Ok(());
18131815
}
18141816

1817+
// we don't care to report errors for a struct if the struct itself is tainted
1818+
variant.has_errors()?;
1819+
18151820
let path = rustc_hir_pretty::qpath_to_string(&self.tcx, qpath);
18161821
let mut err = struct_span_code_err!(
18171822
self.dcx(),

‎compiler/rustc_metadata/src/rmeta/decoder.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1133,7 +1133,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
11331133
.collect(),
11341134
adt_kind,
11351135
parent_did,
1136-
false,
1136+
None,
11371137
data.is_non_exhaustive,
11381138
// FIXME: unnamed fields in crate metadata is unimplemented yet.
11391139
false,

‎compiler/rustc_middle/src/ty/mod.rs

+38-19
Original file line numberDiff line numberDiff line change
@@ -1159,11 +1159,8 @@ bitflags::bitflags! {
11591159
const NO_VARIANT_FLAGS = 0;
11601160
/// Indicates whether the field list of this variant is `#[non_exhaustive]`.
11611161
const IS_FIELD_LIST_NON_EXHAUSTIVE = 1 << 0;
1162-
/// Indicates whether this variant was obtained as part of recovering from
1163-
/// a syntactic error. May be incomplete or bogus.
1164-
const IS_RECOVERED = 1 << 1;
11651162
/// Indicates whether this variant has unnamed fields.
1166-
const HAS_UNNAMED_FIELDS = 1 << 2;
1163+
const HAS_UNNAMED_FIELDS = 1 << 1;
11671164
}
11681165
}
11691166
rustc_data_structures::external_bitflags_debug! { VariantFlags }
@@ -1183,6 +1180,8 @@ pub struct VariantDef {
11831180
pub discr: VariantDiscr,
11841181
/// Fields of this variant.
11851182
pub fields: IndexVec<FieldIdx, FieldDef>,
1183+
/// The error guarantees from parser, if any.
1184+
tainted: Option<ErrorGuaranteed>,
11861185
/// Flags of the variant (e.g. is field list non-exhaustive)?
11871186
flags: VariantFlags,
11881187
}
@@ -1212,7 +1211,7 @@ impl VariantDef {
12121211
fields: IndexVec<FieldIdx, FieldDef>,
12131212
adt_kind: AdtKind,
12141213
parent_did: DefId,
1215-
recovered: bool,
1214+
recover_tainted: Option<ErrorGuaranteed>,
12161215
is_field_list_non_exhaustive: bool,
12171216
has_unnamed_fields: bool,
12181217
) -> Self {
@@ -1227,15 +1226,19 @@ impl VariantDef {
12271226
flags |= VariantFlags::IS_FIELD_LIST_NON_EXHAUSTIVE;
12281227
}
12291228

1230-
if recovered {
1231-
flags |= VariantFlags::IS_RECOVERED;
1232-
}
1233-
12341229
if has_unnamed_fields {
12351230
flags |= VariantFlags::HAS_UNNAMED_FIELDS;
12361231
}
12371232

1238-
VariantDef { def_id: variant_did.unwrap_or(parent_did), ctor, name, discr, fields, flags }
1233+
VariantDef {
1234+
def_id: variant_did.unwrap_or(parent_did),
1235+
ctor,
1236+
name,
1237+
discr,
1238+
fields,
1239+
flags,
1240+
tainted: recover_tainted,
1241+
}
12391242
}
12401243

12411244
/// Is this field list non-exhaustive?
@@ -1244,12 +1247,6 @@ impl VariantDef {
12441247
self.flags.intersects(VariantFlags::IS_FIELD_LIST_NON_EXHAUSTIVE)
12451248
}
12461249

1247-
/// Was this variant obtained as part of recovering from a syntactic error?
1248-
#[inline]
1249-
pub fn is_recovered(&self) -> bool {
1250-
self.flags.intersects(VariantFlags::IS_RECOVERED)
1251-
}
1252-
12531250
/// Does this variant contains unnamed fields
12541251
#[inline]
12551252
pub fn has_unnamed_fields(&self) -> bool {
@@ -1261,6 +1258,12 @@ impl VariantDef {
12611258
Ident::new(self.name, tcx.def_ident_span(self.def_id).unwrap())
12621259
}
12631260

1261+
/// Was this variant obtained as part of recovering from a syntactic error?
1262+
#[inline]
1263+
pub fn has_errors(&self) -> Result<(), ErrorGuaranteed> {
1264+
self.tainted.map_or(Ok(()), Err)
1265+
}
1266+
12641267
#[inline]
12651268
pub fn ctor_kind(&self) -> Option<CtorKind> {
12661269
self.ctor.map(|(kind, _)| kind)
@@ -1308,8 +1311,24 @@ impl PartialEq for VariantDef {
13081311
// definition of `VariantDef` changes, a compile-error will be produced,
13091312
// reminding us to revisit this assumption.
13101313

1311-
let Self { def_id: lhs_def_id, ctor: _, name: _, discr: _, fields: _, flags: _ } = &self;
1312-
let Self { def_id: rhs_def_id, ctor: _, name: _, discr: _, fields: _, flags: _ } = other;
1314+
let Self {
1315+
def_id: lhs_def_id,
1316+
ctor: _,
1317+
name: _,
1318+
discr: _,
1319+
fields: _,
1320+
flags: _,
1321+
tainted: _,
1322+
} = &self;
1323+
let Self {
1324+
def_id: rhs_def_id,
1325+
ctor: _,
1326+
name: _,
1327+
discr: _,
1328+
fields: _,
1329+
flags: _,
1330+
tainted: _,
1331+
} = other;
13131332

13141333
let res = lhs_def_id == rhs_def_id;
13151334

@@ -1339,7 +1358,7 @@ impl Hash for VariantDef {
13391358
// of `VariantDef` changes, a compile-error will be produced, reminding
13401359
// us to revisit this assumption.
13411360

1342-
let Self { def_id, ctor: _, name: _, discr: _, fields: _, flags: _ } = &self;
1361+
let Self { def_id, ctor: _, name: _, discr: _, fields: _, flags: _, tainted: _ } = &self;
13431362
def_id.hash(s)
13441363
}
13451364
}

‎tests/ui/pattern/struct-parser-recovery-issue-126344.rs

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
struct Wrong {
2+
x: i32; //~ ERROR struct fields are separated by `,`
3+
y: i32,
4+
z: i32,
5+
h: i32,
6+
}
7+
8+
fn oops(w: &Wrong) {
9+
w.x;
10+
}
11+
12+
fn foo(w: &Wrong) {
13+
w.y;
14+
}
15+
16+
fn haha(w: &Wrong) {
17+
w.z;
18+
}
19+
20+
struct WrongWithType {
21+
x: 1, //~ ERROR expected type, found `1`
22+
y: i32,
23+
z: i32,
24+
h: i32,
25+
}
26+
27+
fn oops_type(w: &WrongWithType) {
28+
w.x;
29+
}
30+
31+
fn foo_type(w: &WrongWithType) {
32+
w.y;
33+
}
34+
35+
fn haha_type(w: &WrongWithType) {
36+
w.z;
37+
}
38+
39+
fn main() {
40+
let v = Wrong { x: 1, y: 2, z: 3, h: 4 };
41+
let x = WrongWithType { x: 1, y: 2, z: 3, h: 4 };
42+
}

‎tests/ui/pattern/struct-parser-recovery-issue-126344.stderr

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error: struct fields are separated by `,`
2+
--> $DIR/struct-parser-recovery-issue-126344.rs:2:11
3+
|
4+
LL | struct Wrong {
5+
| ----- while parsing this struct
6+
LL | x: i32;
7+
| ^ help: replace `;` with `,`
8+
9+
error: expected type, found `1`
10+
--> $DIR/struct-parser-recovery-issue-126344.rs:21:8
11+
|
12+
LL | struct WrongWithType {
13+
| ------------- while parsing this struct
14+
LL | x: 1,
15+
| ^ expected type
16+
17+
error: aborting due to 2 previous errors
18+

‎tests/ui/thir-print/thir-tree-match.stdout

+4-4
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ body:
9292
adt_def:
9393
AdtDef {
9494
did: DefId(0:10 ~ thir_tree_match[fcf8]::Foo)
95-
variants: [VariantDef { def_id: DefId(0:11 ~ thir_tree_match[fcf8]::Foo::FooOne), ctor: Some((Fn, DefId(0:12 ~ thir_tree_match[fcf8]::Foo::FooOne::{constructor#0}))), name: "FooOne", discr: Relative(0), fields: [FieldDef { did: DefId(0:13 ~ thir_tree_match[fcf8]::Foo::FooOne::0), name: "0", vis: Restricted(DefId(0:0 ~ thir_tree_match[fcf8])) }], flags: }, VariantDef { def_id: DefId(0:14 ~ thir_tree_match[fcf8]::Foo::FooTwo), ctor: Some((Const, DefId(0:15 ~ thir_tree_match[fcf8]::Foo::FooTwo::{constructor#0}))), name: "FooTwo", discr: Relative(1), fields: [], flags: }]
95+
variants: [VariantDef { def_id: DefId(0:11 ~ thir_tree_match[fcf8]::Foo::FooOne), ctor: Some((Fn, DefId(0:12 ~ thir_tree_match[fcf8]::Foo::FooOne::{constructor#0}))), name: "FooOne", discr: Relative(0), fields: [FieldDef { did: DefId(0:13 ~ thir_tree_match[fcf8]::Foo::FooOne::0), name: "0", vis: Restricted(DefId(0:0 ~ thir_tree_match[fcf8])) }], tainted: None, flags: }, VariantDef { def_id: DefId(0:14 ~ thir_tree_match[fcf8]::Foo::FooTwo), ctor: Some((Const, DefId(0:15 ~ thir_tree_match[fcf8]::Foo::FooTwo::{constructor#0}))), name: "FooTwo", discr: Relative(1), fields: [], tainted: None, flags: }]
9696
flags: IS_ENUM
9797
repr: ReprOptions { int: None, align: None, pack: None, flags: , field_shuffle_seed: 3477539199540094892 }
9898
args: []
@@ -106,7 +106,7 @@ body:
106106
adt_def:
107107
AdtDef {
108108
did: DefId(0:3 ~ thir_tree_match[fcf8]::Bar)
109-
variants: [VariantDef { def_id: DefId(0:4 ~ thir_tree_match[fcf8]::Bar::First), ctor: Some((Const, DefId(0:5 ~ thir_tree_match[fcf8]::Bar::First::{constructor#0}))), name: "First", discr: Relative(0), fields: [], flags: }, VariantDef { def_id: DefId(0:6 ~ thir_tree_match[fcf8]::Bar::Second), ctor: Some((Const, DefId(0:7 ~ thir_tree_match[fcf8]::Bar::Second::{constructor#0}))), name: "Second", discr: Relative(1), fields: [], flags: }, VariantDef { def_id: DefId(0:8 ~ thir_tree_match[fcf8]::Bar::Third), ctor: Some((Const, DefId(0:9 ~ thir_tree_match[fcf8]::Bar::Third::{constructor#0}))), name: "Third", discr: Relative(2), fields: [], flags: }]
109+
variants: [VariantDef { def_id: DefId(0:4 ~ thir_tree_match[fcf8]::Bar::First), ctor: Some((Const, DefId(0:5 ~ thir_tree_match[fcf8]::Bar::First::{constructor#0}))), name: "First", discr: Relative(0), fields: [], tainted: None, flags: }, VariantDef { def_id: DefId(0:6 ~ thir_tree_match[fcf8]::Bar::Second), ctor: Some((Const, DefId(0:7 ~ thir_tree_match[fcf8]::Bar::Second::{constructor#0}))), name: "Second", discr: Relative(1), fields: [], tainted: None, flags: }, VariantDef { def_id: DefId(0:8 ~ thir_tree_match[fcf8]::Bar::Third), ctor: Some((Const, DefId(0:9 ~ thir_tree_match[fcf8]::Bar::Third::{constructor#0}))), name: "Third", discr: Relative(2), fields: [], tainted: None, flags: }]
110110
flags: IS_ENUM
111111
repr: ReprOptions { int: None, align: None, pack: None, flags: , field_shuffle_seed: 10333377570083945360 }
112112
args: []
@@ -154,7 +154,7 @@ body:
154154
adt_def:
155155
AdtDef {
156156
did: DefId(0:10 ~ thir_tree_match[fcf8]::Foo)
157-
variants: [VariantDef { def_id: DefId(0:11 ~ thir_tree_match[fcf8]::Foo::FooOne), ctor: Some((Fn, DefId(0:12 ~ thir_tree_match[fcf8]::Foo::FooOne::{constructor#0}))), name: "FooOne", discr: Relative(0), fields: [FieldDef { did: DefId(0:13 ~ thir_tree_match[fcf8]::Foo::FooOne::0), name: "0", vis: Restricted(DefId(0:0 ~ thir_tree_match[fcf8])) }], flags: }, VariantDef { def_id: DefId(0:14 ~ thir_tree_match[fcf8]::Foo::FooTwo), ctor: Some((Const, DefId(0:15 ~ thir_tree_match[fcf8]::Foo::FooTwo::{constructor#0}))), name: "FooTwo", discr: Relative(1), fields: [], flags: }]
157+
variants: [VariantDef { def_id: DefId(0:11 ~ thir_tree_match[fcf8]::Foo::FooOne), ctor: Some((Fn, DefId(0:12 ~ thir_tree_match[fcf8]::Foo::FooOne::{constructor#0}))), name: "FooOne", discr: Relative(0), fields: [FieldDef { did: DefId(0:13 ~ thir_tree_match[fcf8]::Foo::FooOne::0), name: "0", vis: Restricted(DefId(0:0 ~ thir_tree_match[fcf8])) }], tainted: None, flags: }, VariantDef { def_id: DefId(0:14 ~ thir_tree_match[fcf8]::Foo::FooTwo), ctor: Some((Const, DefId(0:15 ~ thir_tree_match[fcf8]::Foo::FooTwo::{constructor#0}))), name: "FooTwo", discr: Relative(1), fields: [], tainted: None, flags: }]
158158
flags: IS_ENUM
159159
repr: ReprOptions { int: None, align: None, pack: None, flags: , field_shuffle_seed: 3477539199540094892 }
160160
args: []
@@ -206,7 +206,7 @@ body:
206206
adt_def:
207207
AdtDef {
208208
did: DefId(0:10 ~ thir_tree_match[fcf8]::Foo)
209-
variants: [VariantDef { def_id: DefId(0:11 ~ thir_tree_match[fcf8]::Foo::FooOne), ctor: Some((Fn, DefId(0:12 ~ thir_tree_match[fcf8]::Foo::FooOne::{constructor#0}))), name: "FooOne", discr: Relative(0), fields: [FieldDef { did: DefId(0:13 ~ thir_tree_match[fcf8]::Foo::FooOne::0), name: "0", vis: Restricted(DefId(0:0 ~ thir_tree_match[fcf8])) }], flags: }, VariantDef { def_id: DefId(0:14 ~ thir_tree_match[fcf8]::Foo::FooTwo), ctor: Some((Const, DefId(0:15 ~ thir_tree_match[fcf8]::Foo::FooTwo::{constructor#0}))), name: "FooTwo", discr: Relative(1), fields: [], flags: }]
209+
variants: [VariantDef { def_id: DefId(0:11 ~ thir_tree_match[fcf8]::Foo::FooOne), ctor: Some((Fn, DefId(0:12 ~ thir_tree_match[fcf8]::Foo::FooOne::{constructor#0}))), name: "FooOne", discr: Relative(0), fields: [FieldDef { did: DefId(0:13 ~ thir_tree_match[fcf8]::Foo::FooOne::0), name: "0", vis: Restricted(DefId(0:0 ~ thir_tree_match[fcf8])) }], tainted: None, flags: }, VariantDef { def_id: DefId(0:14 ~ thir_tree_match[fcf8]::Foo::FooTwo), ctor: Some((Const, DefId(0:15 ~ thir_tree_match[fcf8]::Foo::FooTwo::{constructor#0}))), name: "FooTwo", discr: Relative(1), fields: [], tainted: None, flags: }]
210210
flags: IS_ENUM
211211
repr: ReprOptions { int: None, align: None, pack: None, flags: , field_shuffle_seed: 3477539199540094892 }
212212
args: []

‎tests/crashes/126744.rs ‎tests/ui/typeck/struct-index-err-ice-issue-126744.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
//@ known-bug: rust-lang/rust#126744
2-
struct X {,}
1+
struct X {,} //~ ERROR expected identifier, found `,`
32

43
fn main() {
54
|| {

‎tests/ui/typeck/struct-index-err-ice-issue-126744.stderr

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
error: expected identifier, found `,`
2+
--> $DIR/struct-index-err-ice-issue-126744.rs:1:11
3+
|
4+
LL | struct X {,}
5+
| - ^ expected identifier
6+
| |
7+
| while parsing this struct
8+
9+
error: aborting due to 1 previous error
10+

0 commit comments

Comments
 (0)