Rollup merge of rust-lang#138679 - Shunpoco:issue-125323, r=oli-obk

Issue-125323: ICE non-ADT in struct pattern when long time constant evaluation is in for loop

This PR fixes rust-lang#125323

## Context
According to the issue, the ICE happens since rust-lang#121206.
In the PR, some error methods were reorganized. For example, has_errors() was renamed to has_errors_exclude_lint_errors(). However, some codes which used the original has_errors() were not switched to has_errors_exclude_lint_errors(). I finally found that report_error() in writeback.rs causes this ICE. Currently the method uses tainted_by_errors() to get guar (ErrorGuaranteed), but originally it used dcx().has_errors() but it wasn't changed to has_errors_exclude_lint_errors() when changes in rust-lang#121206 were merged. I don't think I fully understand how an error is propagated, but I suppose that the error from long time constant evaluation is unexpectedly propagated other parts (in this ICE, for loop), then cause the non-ADT in struct pattern ICE.

## Change
- Fix report_error() in writeback.rs: use dcx().has_errors_exclude_lint_errors() instead of tainted_by_errors() to prevent error propagation from constant evaluation.
- Add test for the ICE
- Modify some tests to align the change: Due to this fix, E0282 error happens (or not happen anymore) in some tests.

## NOTE
The 4th commit aims to revert the fix in rust-lang#123516 because I confirmed that the ICE solved by the PR doesn't happen if I modify report_error(). I think the root cause of that ICE is the same as rust-lang#125323 . But I can discard this commit since we can fix rust-lang#125323 without it.

Author: matthiaskrgr

compiler/rustc_lint/src/builtin.rs

@@ -152,7 +152,10 @@ declare_lint_pass!(NonShorthandFieldPatterns => [NON_SHORTHAND_FIELD_PATTERNS]);
 
 impl<'tcx> LateLintPass<'tcx> for NonShorthandFieldPatterns {
     fn check_pat(&mut self, cx: &LateContext<'_>, pat: &hir::Pat<'_>) {
-        if let PatKind::Struct(ref qpath, field_pats, _) = pat.kind {
+        // The result shouldn't be tainted, otherwise it will cause ICE.
+        if let PatKind::Struct(ref qpath, field_pats, _) = pat.kind
+            && cx.typeck_results().tainted_by_errors.is_none()
+        {
             let variant = cx
                 .typeck_results()
                 .pat_ty(pat)

compiler/rustc_middle/src/query/mod.rs

@@ -1202,10 +1202,10 @@ rustc_queries! {
     /// Return the live symbols in the crate for dead code check.
     ///
     /// The second return value maps from ADTs to ignored derived traits (e.g. Debug and Clone).
-    query live_symbols_and_ignored_derived_traits(_: ()) -> &'tcx (
+    query live_symbols_and_ignored_derived_traits(_: ()) -> &'tcx Result<(
         LocalDefIdSet,
         LocalDefIdMap<FxIndexSet<DefId>>,
-    ) {
+    ), ErrorGuaranteed> {
         arena_cache
         desc { "finding live symbols in crate" }
     }

compiler/rustc_passes/src/dead.rs

@@ -4,11 +4,12 @@
 // is dead.
 
 use std::mem;
+use std::ops::ControlFlow;
 
 use hir::def_id::{LocalDefIdMap, LocalDefIdSet};
 use rustc_abi::FieldIdx;
 use rustc_data_structures::fx::FxIndexSet;
-use rustc_errors::MultiSpan;
+use rustc_errors::{ErrorGuaranteed, MultiSpan};
 use rustc_hir::def::{CtorOf, DefKind, Res};
 use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId};
 use rustc_hir::intravisit::{self, Visitor};
@@ -178,12 +179,12 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
             .iter()
             .any(|adj| matches!(adj.kind, ty::adjustment::Adjust::Deref(_)))
         {
-            self.visit_expr(expr);
+            let _ = self.visit_expr(expr);
         } else if let hir::ExprKind::Field(base, ..) = expr.kind {
             // Ignore write to field
             self.handle_assign(base);
         } else {
-            self.visit_expr(expr);
+            let _ = self.visit_expr(expr);
         }
     }
 
@@ -318,7 +319,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
         }
     }
 
-    fn mark_live_symbols(&mut self) {
+    fn mark_live_symbols(&mut self) -> <MarkSymbolVisitor<'tcx> as Visitor<'tcx>>::Result {
         while let Some(work) = self.worklist.pop() {
             let (mut id, comes_from_allow_expect) = work;
 
@@ -366,8 +367,10 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
                 continue;
             }
 
-            self.visit_node(self.tcx.hir_node_by_def_id(id));
+            self.visit_node(self.tcx.hir_node_by_def_id(id))?;
         }
+
+        ControlFlow::Continue(())
     }
 
     /// Automatically generated items marked with `rustc_trivial_field_reads`
@@ -391,19 +394,22 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
         false
     }
 
-    fn visit_node(&mut self, node: Node<'tcx>) {
+    fn visit_node(
+        &mut self,
+        node: Node<'tcx>,
+    ) -> <MarkSymbolVisitor<'tcx> as Visitor<'tcx>>::Result {
         if let Node::ImplItem(impl_item) = node
             && self.should_ignore_impl_item(impl_item)
         {
-            return;
+            return ControlFlow::Continue(());
         }
 
         let unconditionally_treated_fields_as_live =
             self.repr_unconditionally_treats_fields_as_live;
         let had_repr_simd = self.repr_has_repr_simd;
         self.repr_unconditionally_treats_fields_as_live = false;
         self.repr_has_repr_simd = false;
-        match node {
+        let walk_result = match node {
             Node::Item(item) => match item.kind {
                 hir::ItemKind::Struct(..) | hir::ItemKind::Union(..) => {
                     let def = self.tcx.adt_def(item.owner_id);
@@ -413,7 +419,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
 
                     intravisit::walk_item(self, item)
                 }
-                hir::ItemKind::ForeignMod { .. } => {}
+                hir::ItemKind::ForeignMod { .. } => ControlFlow::Continue(()),
                 hir::ItemKind::Trait(.., trait_item_refs) => {
                     // mark assoc ty live if the trait is live
                     for trait_item in trait_item_refs {
@@ -431,7 +437,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
                 if let Some(trait_id) = self.tcx.trait_of_assoc(trait_item_id) {
                     self.check_def_id(trait_id);
                 }
-                intravisit::walk_trait_item(self, trait_item);
+                intravisit::walk_trait_item(self, trait_item)
             }
             Node::ImplItem(impl_item) => {
                 let item = self.tcx.local_parent(impl_item.owner_id.def_id);
@@ -452,16 +458,16 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
                         _ => {}
                     }
                 }
-                intravisit::walk_impl_item(self, impl_item);
-            }
-            Node::ForeignItem(foreign_item) => {
-                intravisit::walk_foreign_item(self, foreign_item);
+                intravisit::walk_impl_item(self, impl_item)
             }
+            Node::ForeignItem(foreign_item) => intravisit::walk_foreign_item(self, foreign_item),
             Node::OpaqueTy(opaq) => intravisit::walk_opaque_ty(self, opaq),
-            _ => {}
-        }
+            _ => ControlFlow::Continue(()),
+        };
         self.repr_has_repr_simd = had_repr_simd;
         self.repr_unconditionally_treats_fields_as_live = unconditionally_treated_fields_as_live;
+
+        walk_result
     }
 
     fn mark_as_used_if_union(&mut self, adt: ty::AdtDef<'tcx>, fields: &[hir::ExprField<'_>]) {
@@ -511,15 +517,25 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
 }
 
 impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
-    fn visit_nested_body(&mut self, body: hir::BodyId) {
-        let old_maybe_typeck_results =
-            self.maybe_typeck_results.replace(self.tcx.typeck_body(body));
+    type Result = ControlFlow<ErrorGuaranteed>;
+
+    fn visit_nested_body(&mut self, body: hir::BodyId) -> Self::Result {
+        let typeck_results = self.tcx.typeck_body(body);
+
+        // The result shouldn't be tainted, otherwise it will cause ICE.
+        if let Some(guar) = typeck_results.tainted_by_errors {
+            return ControlFlow::Break(guar);
+        }
+
+        let old_maybe_typeck_results = self.maybe_typeck_results.replace(typeck_results);
         let body = self.tcx.hir_body(body);
-        self.visit_body(body);
+        let result = self.visit_body(body);
         self.maybe_typeck_results = old_maybe_typeck_results;
+
+        result
     }
 
-    fn visit_variant_data(&mut self, def: &'tcx hir::VariantData<'tcx>) {
+    fn visit_variant_data(&mut self, def: &'tcx hir::VariantData<'tcx>) -> Self::Result {
         let tcx = self.tcx;
         let unconditionally_treat_fields_as_live = self.repr_unconditionally_treats_fields_as_live;
         let has_repr_simd = self.repr_has_repr_simd;
@@ -536,10 +552,10 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
         });
         self.live_symbols.extend(live_fields);
 
-        intravisit::walk_struct_def(self, def);
+        intravisit::walk_struct_def(self, def)
     }
 
-    fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
+    fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) -> Self::Result {
         match expr.kind {
             hir::ExprKind::Path(ref qpath @ QPath::TypeRelative(..)) => {
                 let res = self.typeck_results().qpath_res(qpath, expr.hir_id);
@@ -575,20 +591,22 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
             _ => (),
         }
 
-        intravisit::walk_expr(self, expr);
+        intravisit::walk_expr(self, expr)
     }
 
-    fn visit_arm(&mut self, arm: &'tcx hir::Arm<'tcx>) {
+    fn visit_arm(&mut self, arm: &'tcx hir::Arm<'tcx>) -> Self::Result {
         // Inside the body, ignore constructions of variants
         // necessary for the pattern to match. Those construction sites
         // can't be reached unless the variant is constructed elsewhere.
         let len = self.ignore_variant_stack.len();
         self.ignore_variant_stack.extend(arm.pat.necessary_variants());
-        intravisit::walk_arm(self, arm);
+        let result = intravisit::walk_arm(self, arm);
         self.ignore_variant_stack.truncate(len);
+
+        result
     }
 
-    fn visit_pat(&mut self, pat: &'tcx hir::Pat<'tcx>) {
+    fn visit_pat(&mut self, pat: &'tcx hir::Pat<'tcx>) -> Self::Result {
         self.in_pat = true;
         match pat.kind {
             PatKind::Struct(ref path, fields, _) => {
@@ -602,11 +620,13 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
             _ => (),
         }
 
-        intravisit::walk_pat(self, pat);
+        let result = intravisit::walk_pat(self, pat);
         self.in_pat = false;
+
+        result
     }
 
-    fn visit_pat_expr(&mut self, expr: &'tcx rustc_hir::PatExpr<'tcx>) {
+    fn visit_pat_expr(&mut self, expr: &'tcx rustc_hir::PatExpr<'tcx>) -> Self::Result {
         match &expr.kind {
             rustc_hir::PatExprKind::Path(qpath) => {
                 // mark the type of variant live when meeting E::V in expr
@@ -619,37 +639,41 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
             }
             _ => {}
         }
-        intravisit::walk_pat_expr(self, expr);
+        intravisit::walk_pat_expr(self, expr)
     }
 
-    fn visit_path(&mut self, path: &hir::Path<'tcx>, _: hir::HirId) {
+    fn visit_path(&mut self, path: &hir::Path<'tcx>, _: hir::HirId) -> Self::Result {
         self.handle_res(path.res);
-        intravisit::walk_path(self, path);
+        intravisit::walk_path(self, path)
     }
 
-    fn visit_anon_const(&mut self, c: &'tcx hir::AnonConst) {
+    fn visit_anon_const(&mut self, c: &'tcx hir::AnonConst) -> Self::Result {
         // When inline const blocks are used in pattern position, paths
         // referenced by it should be considered as used.
         let in_pat = mem::replace(&mut self.in_pat, false);
 
         self.live_symbols.insert(c.def_id);
-        intravisit::walk_anon_const(self, c);
+        let result = intravisit::walk_anon_const(self, c);
 
         self.in_pat = in_pat;
+
+        result
     }
 
-    fn visit_inline_const(&mut self, c: &'tcx hir::ConstBlock) {
+    fn visit_inline_const(&mut self, c: &'tcx hir::ConstBlock) -> Self::Result {
         // When inline const blocks are used in pattern position, paths
         // referenced by it should be considered as used.
         let in_pat = mem::replace(&mut self.in_pat, false);
 
         self.live_symbols.insert(c.def_id);
-        intravisit::walk_inline_const(self, c);
+        let result = intravisit::walk_inline_const(self, c);
 
         self.in_pat = in_pat;
+
+        result
     }
 
-    fn visit_trait_ref(&mut self, t: &'tcx hir::TraitRef<'tcx>) {
+    fn visit_trait_ref(&mut self, t: &'tcx hir::TraitRef<'tcx>) -> Self::Result {
         if let Some(trait_def_id) = t.path.res.opt_def_id()
             && let Some(segment) = t.path.segments.last()
             && let Some(args) = segment.args
@@ -671,7 +695,7 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
             }
         }
 
-        intravisit::walk_trait_ref(self, t);
+        intravisit::walk_trait_ref(self, t)
     }
 }
 
@@ -818,7 +842,7 @@ fn create_and_seed_worklist(
 fn live_symbols_and_ignored_derived_traits(
     tcx: TyCtxt<'_>,
     (): (),
-) -> (LocalDefIdSet, LocalDefIdMap<FxIndexSet<DefId>>) {
+) -> Result<(LocalDefIdSet, LocalDefIdMap<FxIndexSet<DefId>>), ErrorGuaranteed> {
     let (worklist, mut unsolved_items) = create_and_seed_worklist(tcx);
     let mut symbol_visitor = MarkSymbolVisitor {
         worklist,
@@ -832,7 +856,9 @@ fn live_symbols_and_ignored_derived_traits(
         ignore_variant_stack: vec![],
         ignored_derived_traits: Default::default(),
     };
-    symbol_visitor.mark_live_symbols();
+    if let ControlFlow::Break(guar) = symbol_visitor.mark_live_symbols() {
+        return Err(guar);
+    }
 
     // We have marked the primary seeds as live. We now need to process unsolved items from traits
     // and trait impls: add them to the work list if the trait or the implemented type is live.
@@ -846,14 +872,16 @@ fn live_symbols_and_ignored_derived_traits(
         symbol_visitor
             .worklist
             .extend(items_to_check.drain(..).map(|id| (id, ComesFromAllowExpect::No)));
-        symbol_visitor.mark_live_symbols();
+        if let ControlFlow::Break(guar) = symbol_visitor.mark_live_symbols() {
+            return Err(guar);
+        }
 
         items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| {
             symbol_visitor.check_impl_or_impl_item_live(local_def_id)
         }));
     }
 
-    (symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits)
+    Ok((symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits))
 }
 
 struct DeadItem {
@@ -1133,7 +1161,12 @@ impl<'tcx> DeadVisitor<'tcx> {
 }
 
 fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
-    let (live_symbols, ignored_derived_traits) = tcx.live_symbols_and_ignored_derived_traits(());
+    let Ok((live_symbols, ignored_derived_traits)) =
+        tcx.live_symbols_and_ignored_derived_traits(()).as_ref()
+    else {
+        return;
+    };
+
     let mut visitor = DeadVisitor { tcx, live_symbols, ignored_derived_traits };
 
     let module_items = tcx.hir_module_items(module);

tests/crashes/125323.rs

@@ -0,0 +1,14 @@
+// The test confirms ICE-125323 is fixed.
+//
+// This warning tests there is no warning about dead code
+// when there is a constant evaluation error.
+#![warn(unused)]
+fn should_not_be_dead() {}
+
+fn main() {
+    for _ in 0..0 {
+        [(); loop {}]; //~ ERROR constant evaluation is taking a long time
+    }
+
+    should_not_be_dead();
+}

tests/ui/consts/do-not-ice-long-constant-evaluation-in-for-loop.rs

@@ -0,0 +1,17 @@
+error: constant evaluation is taking a long time
+  --> $DIR/do-not-ice-long-constant-evaluation-in-for-loop.rs:10:14
+   |
+LL |         [(); loop {}];
+   |              ^^^^^^^
+   |
+   = note: this lint makes sure the compiler doesn't get stuck due to infinite loops in const eval.
+           If your compilation actually takes a long time, you can safely allow the lint.
+help: the constant being evaluated
+  --> $DIR/do-not-ice-long-constant-evaluation-in-for-loop.rs:10:14
+   |
+LL |         [(); loop {}];
+   |              ^^^^^^^
+   = note: `#[deny(long_running_const_eval)]` on by default
+
+error: aborting due to 1 previous error
+