From 07e6dd95bd84bd506a0a04f2054ff82c681be928 Mon Sep 17 00:00:00 2001 From: yukang Date: Thu, 11 Jul 2024 00:07:07 +0800 Subject: [PATCH] report pat no field error no recoverd struct variant --- compiler/rustc_hir_analysis/src/collect.rs | 5 +- compiler/rustc_hir_typeck/src/expr.rs | 11 ++-- compiler/rustc_hir_typeck/src/pat.rs | 7 ++- compiler/rustc_metadata/src/rmeta/decoder.rs | 2 +- compiler/rustc_middle/src/ty/mod.rs | 57 ++++++++++++------- .../struct-parser-recovery-issue-126344.rs | 42 ++++++++++++++ ...struct-parser-recovery-issue-126344.stderr | 18 ++++++ tests/ui/thir-print/thir-tree-match.stdout | 8 +-- .../struct-index-err-ice-issue-126744.rs} | 3 +- .../struct-index-err-ice-issue-126744.stderr | 10 ++++ 10 files changed, 131 insertions(+), 32 deletions(-) create mode 100644 tests/ui/pattern/struct-parser-recovery-issue-126344.rs create mode 100644 tests/ui/pattern/struct-parser-recovery-issue-126344.stderr rename tests/{crashes/126744.rs => ui/typeck/struct-index-err-ice-issue-126744.rs} (56%) create mode 100644 tests/ui/typeck/struct-index-err-ice-issue-126744.stderr diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index 843e4d41e00..b95ea60e49d 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -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), diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index bd5e5294983..0a1294964d0 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -2177,10 +2177,8 @@ fn report_unknown_field( 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( @@ -2345,6 +2343,11 @@ fn check_field( 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]; diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs index be526e1c26c..6d1e9ff1f95 100644 --- a/compiler/rustc_hir_typeck/src/pat.rs +++ b/compiler/rustc_hir_typeck/src/pat.rs @@ -1531,9 +1531,11 @@ fn check_struct_pat_fields( .filter(|(_, ident)| !used_fields.contains_key(ident)) .collect::>(); - 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, @@ -1812,6 +1814,9 @@ fn error_tuple_variant_as_struct_pat( 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(), diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 83b41e0540e..0ba9b940eed 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -1133,7 +1133,7 @@ fn get_variant( .collect(), adt_kind, parent_did, - false, + None, data.is_non_exhaustive, // FIXME: unnamed fields in crate metadata is unimplemented yet. false, diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 4470db47474..48d4d702513 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -1159,11 +1159,8 @@ impl VariantFlags: u8 { 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 } @@ -1183,6 +1180,8 @@ pub struct VariantDef { pub discr: VariantDiscr, /// Fields of this variant. pub fields: IndexVec, + /// The error guarantees from parser, if any. + tainted: Option, /// Flags of the variant (e.g. is field list non-exhaustive)? flags: VariantFlags, } @@ -1212,7 +1211,7 @@ pub fn new( fields: IndexVec, adt_kind: AdtKind, parent_did: DefId, - recovered: bool, + recover_tainted: Option, is_field_list_non_exhaustive: bool, has_unnamed_fields: bool, ) -> Self { @@ -1227,15 +1226,19 @@ pub fn new( 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? @@ -1244,12 +1247,6 @@ pub fn is_field_list_non_exhaustive(&self) -> bool { 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 { @@ -1261,6 +1258,12 @@ pub fn ident(&self, tcx: TyCtxt<'_>) -> Ident { 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 { self.ctor.map(|(kind, _)| kind) @@ -1308,8 +1311,24 @@ fn eq(&self, other: &Self) -> bool { // 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; @@ -1339,7 +1358,7 @@ fn hash(&self, s: &mut H) { // 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) } } diff --git a/tests/ui/pattern/struct-parser-recovery-issue-126344.rs b/tests/ui/pattern/struct-parser-recovery-issue-126344.rs new file mode 100644 index 00000000000..1e3ce3e025e --- /dev/null +++ b/tests/ui/pattern/struct-parser-recovery-issue-126344.rs @@ -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 }; +} diff --git a/tests/ui/pattern/struct-parser-recovery-issue-126344.stderr b/tests/ui/pattern/struct-parser-recovery-issue-126344.stderr new file mode 100644 index 00000000000..ef7f9c566db --- /dev/null +++ b/tests/ui/pattern/struct-parser-recovery-issue-126344.stderr @@ -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 + diff --git a/tests/ui/thir-print/thir-tree-match.stdout b/tests/ui/thir-print/thir-tree-match.stdout index 31c46716a00..b2431698cc6 100644 --- a/tests/ui/thir-print/thir-tree-match.stdout +++ b/tests/ui/thir-print/thir-tree-match.stdout @@ -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: [] @@ -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: [] @@ -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: [] @@ -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: [] diff --git a/tests/crashes/126744.rs b/tests/ui/typeck/struct-index-err-ice-issue-126744.rs similarity index 56% rename from tests/crashes/126744.rs rename to tests/ui/typeck/struct-index-err-ice-issue-126744.rs index ed562c86e61..00edc4d8e41 100644 --- a/tests/crashes/126744.rs +++ b/tests/ui/typeck/struct-index-err-ice-issue-126744.rs @@ -1,5 +1,4 @@ -//@ known-bug: rust-lang/rust#126744 -struct X {,} +struct X {,} //~ ERROR expected identifier, found `,` fn main() { || { diff --git a/tests/ui/typeck/struct-index-err-ice-issue-126744.stderr b/tests/ui/typeck/struct-index-err-ice-issue-126744.stderr new file mode 100644 index 00000000000..84a3af9ab00 --- /dev/null +++ b/tests/ui/typeck/struct-index-err-ice-issue-126744.stderr @@ -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 +