Auto merge of #127575 - chenyukang:yukang-fix-struct-fields-ice, r=compiler-errors

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

Fixes https://github.com/rust-lang/rust/issues/126744
Fixes https://github.com/rust-lang/rust/issues/126344, a more general fix compared with https://github.com/rust-lang/rust/pull/127426

r? `@oli-obk`

From `@compiler-errors` 's comment https://github.com/rust-lang/rust/pull/127502#discussion_r1669538204
Seems most of the ADTs don't have taint, so maybe it's not proper to change `TyCtxt::type_of` query.

compiler/rustc_hir_analysis/src/collect.rs

@@ -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),

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() {
+        // we don't care to report errors for a struct if the struct itself is tainted
-            let guar =
+        if let Err(guar) = variant.has_errors() {
-                self.dcx().span_delayed_bug(expr.span, "parser recovered but no error was emitted");
-            self.set_tainted_by_errors(guar);
             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];

compiler/rustc_hir_typeck/src/pat.rs

@@ -1531,9 +1531,11 @@ fn check_struct_pat_fields(
             .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,
@@ -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(),

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,

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<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,
 }
@@ -1212,7 +1211,7 @@ pub fn new(
         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 {
@@ -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<CtorKind> {
         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 {
-        let Self { def_id: rhs_def_id, ctor: _, name: _, discr: _, fields: _, flags: _ } = other;
+            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<H: Hasher>(&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)
     }
 }

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 };
+}

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
+

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: []

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

@@ -1,5 +1,4 @@
-//@ known-bug: rust-lang/rust#126744
+struct X {,} //~ ERROR expected identifier, found `,`
-struct X {,}
 
 fn main() {
     || {

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
+