Skip to content

Commit

Permalink
Auto merge of rust-lang#127575 - chenyukang:yukang-fix-struct-fields-…
Browse files Browse the repository at this point in the history 
…ice, r=compiler-errors

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

Fixes rust-lang#126744
Fixes rust-lang#126344, a more general fix compared with rust-lang#127426

r? `@oli-obk`

From `@compiler-errors` 's comment rust-lang#127502 (comment)
Seems most of the ADTs don't have taint, so maybe it's not proper to change `TyCtxt::type_of` query.
  • Loading branch information
@bors
bors committed Jul 11, 2024
2 parents e1f45a1 + 07e6dd9 commit 8c39ac9
Show file tree
Hide file tree
Showing 10 changed files with 131 additions and 32 deletions.
5 changes: 4 additions & 1 deletion compiler/rustc_hir_analysis/src/collect.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1125,7 +1125,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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
3 changes: 1 addition & 2 deletions tests/crashes/126744.rs → ...peck/struct-index-err-ice-issue-126744.rs
View file
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
View file
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

0 comments on commit 8c39ac9

Please sign in to comment.