Skip to content

Avoid "no field" error and ICE on recovered ADT variant #127575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1091,7 +1091,10 @@ fn lower_variant(
vis: tcx.visibility(f.def_id),
})
.collect();
let recovered = matches!(def, hir::VariantData::Struct { recovered: Recovered::Yes(_), .. });
let recovered = match def {
hir::VariantData::Struct { recovered: Recovered::Yes(guar), .. } => Some(*guar),
_ => None,
};
ty::VariantDef::new(
ident.name,
variant_did.map(LocalDefId::to_def_id),
Expand Down
11 changes: 7 additions & 4 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2177,10 +2177,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
skip_fields: &[hir::ExprField<'_>],
kind_name: &str,
) -> ErrorGuaranteed {
if variant.is_recovered() {
let guar =
self.dcx().span_delayed_bug(expr.span, "parser recovered but no error was emitted");
self.set_tainted_by_errors(guar);
// we don't care to report errors for a struct if the struct itself is tainted
if let Err(guar) = variant.has_errors() {
return guar;
}
let mut err = self.err_ctxt().type_error_struct_with_diag(
Expand Down Expand Up @@ -2345,6 +2343,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let mut last_ty = None;
let mut nested_fields = Vec::new();
let mut index = None;

// we don't care to report errors for a struct if the struct itself is tainted
if let Err(guar) = adt_def.non_enum_variant().has_errors() {
return Ty::new_error(self.tcx(), guar);
}
while let Some(idx) = self.tcx.find_field((adt_def.did(), ident)) {
let &mut first_idx = index.get_or_insert(idx);
let field = &adt_def.non_enum_variant().fields[idx];
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_hir_typeck/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1531,9 +1531,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.filter(|(_, ident)| !used_fields.contains_key(ident))
.collect::<Vec<_>>();

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

// we don't care to report errors for a struct if the struct itself is tainted
variant.has_errors()?;

let path = rustc_hir_pretty::qpath_to_string(&self.tcx, qpath);
let mut err = struct_span_code_err!(
self.dcx(),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1133,7 +1133,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
.collect(),
adt_kind,
parent_did,
false,
None,
data.is_non_exhaustive,
// FIXME: unnamed fields in crate metadata is unimplemented yet.
false,
Expand Down
57 changes: 38 additions & 19 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1159,11 +1159,8 @@ bitflags::bitflags! {
const NO_VARIANT_FLAGS = 0;
/// Indicates whether the field list of this variant is `#[non_exhaustive]`.
const IS_FIELD_LIST_NON_EXHAUSTIVE = 1 << 0;
/// Indicates whether this variant was obtained as part of recovering from
/// a syntactic error. May be incomplete or bogus.
const IS_RECOVERED = 1 << 1;
/// Indicates whether this variant has unnamed fields.
const HAS_UNNAMED_FIELDS = 1 << 2;
const HAS_UNNAMED_FIELDS = 1 << 1;
}
}
rustc_data_structures::external_bitflags_debug! { VariantFlags }
Expand All @@ -1183,6 +1180,8 @@ pub struct VariantDef {
pub discr: VariantDiscr,
/// Fields of this variant.
pub fields: IndexVec<FieldIdx, FieldDef>,
/// The error guarantees from parser, if any.
tainted: Option<ErrorGuaranteed>,
/// Flags of the variant (e.g. is field list non-exhaustive)?
flags: VariantFlags,
}
Expand Down Expand Up @@ -1212,7 +1211,7 @@ impl VariantDef {
fields: IndexVec<FieldIdx, FieldDef>,
adt_kind: AdtKind,
parent_did: DefId,
recovered: bool,
recover_tainted: Option<ErrorGuaranteed>,
is_field_list_non_exhaustive: bool,
has_unnamed_fields: bool,
) -> Self {
Expand All @@ -1227,15 +1226,19 @@ impl VariantDef {
flags |= VariantFlags::IS_FIELD_LIST_NON_EXHAUSTIVE;
}

if recovered {
flags |= VariantFlags::IS_RECOVERED;
}

if has_unnamed_fields {
flags |= VariantFlags::HAS_UNNAMED_FIELDS;
}

VariantDef { def_id: variant_did.unwrap_or(parent_did), ctor, name, discr, fields, flags }
VariantDef {
def_id: variant_did.unwrap_or(parent_did),
ctor,
name,
discr,
fields,
flags,
tainted: recover_tainted,
}
}

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

/// Was this variant obtained as part of recovering from a syntactic error?
#[inline]
pub fn is_recovered(&self) -> bool {
self.flags.intersects(VariantFlags::IS_RECOVERED)
}

/// Does this variant contains unnamed fields
#[inline]
pub fn has_unnamed_fields(&self) -> bool {
Expand All @@ -1261,6 +1258,12 @@ impl VariantDef {
Ident::new(self.name, tcx.def_ident_span(self.def_id).unwrap())
}

/// Was this variant obtained as part of recovering from a syntactic error?
#[inline]
pub fn has_errors(&self) -> Result<(), ErrorGuaranteed> {
self.tainted.map_or(Ok(()), Err)
}

#[inline]
pub fn ctor_kind(&self) -> Option<CtorKind> {
self.ctor.map(|(kind, _)| kind)
Expand Down Expand Up @@ -1308,8 +1311,24 @@ impl PartialEq for VariantDef {
// definition of `VariantDef` changes, a compile-error will be produced,
// reminding us to revisit this assumption.

let Self { def_id: lhs_def_id, ctor: _, name: _, discr: _, fields: _, flags: _ } = &self;
let Self { def_id: rhs_def_id, ctor: _, name: _, discr: _, fields: _, flags: _ } = other;
let Self {
def_id: lhs_def_id,
ctor: _,
name: _,
discr: _,
fields: _,
flags: _,
tainted: _,
} = &self;
let Self {
def_id: rhs_def_id,
ctor: _,
name: _,
discr: _,
fields: _,
flags: _,
tainted: _,
} = other;

let res = lhs_def_id == rhs_def_id;

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

let Self { def_id, ctor: _, name: _, discr: _, fields: _, flags: _ } = &self;
let Self { def_id, ctor: _, name: _, discr: _, fields: _, flags: _, tainted: _ } = &self;
def_id.hash(s)
}
}
Expand Down
42 changes: 42 additions & 0 deletions tests/ui/pattern/struct-parser-recovery-issue-126344.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
struct Wrong {
x: i32; //~ ERROR struct fields are separated by `,`
y: i32,
z: i32,
h: i32,
}

fn oops(w: &Wrong) {
w.x;
}

fn foo(w: &Wrong) {
w.y;
}

fn haha(w: &Wrong) {
w.z;
}

struct WrongWithType {
x: 1, //~ ERROR expected type, found `1`
y: i32,
z: i32,
h: i32,
}

fn oops_type(w: &WrongWithType) {
w.x;
}

fn foo_type(w: &WrongWithType) {
w.y;
}

fn haha_type(w: &WrongWithType) {
w.z;
}

fn main() {
let v = Wrong { x: 1, y: 2, z: 3, h: 4 };
let x = WrongWithType { x: 1, y: 2, z: 3, h: 4 };
}
18 changes: 18 additions & 0 deletions tests/ui/pattern/struct-parser-recovery-issue-126344.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error: struct fields are separated by `,`
--> $DIR/struct-parser-recovery-issue-126344.rs:2:11
|
LL | struct Wrong {
| ----- while parsing this struct
LL | x: i32;
| ^ help: replace `;` with `,`

error: expected type, found `1`
--> $DIR/struct-parser-recovery-issue-126344.rs:21:8
|
LL | struct WrongWithType {
| ------------- while parsing this struct
LL | x: 1,
| ^ expected type

error: aborting due to 2 previous errors

8 changes: 4 additions & 4 deletions tests/ui/thir-print/thir-tree-match.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ body:
adt_def:
AdtDef {
did: DefId(0:10 ~ thir_tree_match[fcf8]::Foo)
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: }]
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: }]
flags: IS_ENUM
repr: ReprOptions { int: None, align: None, pack: None, flags: , field_shuffle_seed: 3477539199540094892 }
args: []
Expand All @@ -106,7 +106,7 @@ body:
adt_def:
AdtDef {
did: DefId(0:3 ~ thir_tree_match[fcf8]::Bar)
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: }]
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: }]
flags: IS_ENUM
repr: ReprOptions { int: None, align: None, pack: None, flags: , field_shuffle_seed: 10333377570083945360 }
args: []
Expand Down Expand Up @@ -154,7 +154,7 @@ body:
adt_def:
AdtDef {
did: DefId(0:10 ~ thir_tree_match[fcf8]::Foo)
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: }]
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: }]
flags: IS_ENUM
repr: ReprOptions { int: None, align: None, pack: None, flags: , field_shuffle_seed: 3477539199540094892 }
args: []
Expand Down Expand Up @@ -206,7 +206,7 @@ body:
adt_def:
AdtDef {
did: DefId(0:10 ~ thir_tree_match[fcf8]::Foo)
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: }]
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: }]
flags: IS_ENUM
repr: ReprOptions { int: None, align: None, pack: None, flags: , field_shuffle_seed: 3477539199540094892 }
args: []
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//@ known-bug: rust-lang/rust#126744
struct X {,}
struct X {,} //~ ERROR expected identifier, found `,`

fn main() {
|| {
Expand Down
10 changes: 10 additions & 0 deletions tests/ui/typeck/struct-index-err-ice-issue-126744.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: expected identifier, found `,`
--> $DIR/struct-index-err-ice-issue-126744.rs:1:11
|
LL | struct X {,}
| - ^ expected identifier
| |
| while parsing this struct

error: aborting due to 1 previous error

Loading