Unrolled build for #138679
Rollup merge of #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 #125323
## Context
According to the issue, the ICE happens since #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 #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 #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 #125323 . But I can discard this commit since we can fix #125323 without it.
diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs
index 6167b0d..d9bcd5e 100644
--- a/compiler/rustc_lint/src/builtin.rs
+++ b/compiler/rustc_lint/src/builtin.rs
@@ -152,7 +152,10 @@ fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
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)
diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs
index 151ec4c..588ff68 100644
--- a/compiler/rustc_middle/src/query/mod.rs
+++ b/compiler/rustc_middle/src/query/mod.rs
@@ -1202,10 +1202,10 @@
/// 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" }
}
diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs
index 49dd21f..877b5ad 100644
--- a/compiler/rustc_passes/src/dead.rs
+++ b/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 @@ fn handle_assign(&mut self, expr: &'tcx hir::Expr<'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 @@ fn handle_offset_of(&mut self, expr: &'tcx hir::Expr<'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 @@ fn mark_live_symbols(&mut self) {
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,11 +394,14 @@ fn should_ignore_impl_item(&mut self, impl_item: &hir::ImplItem<'_>) -> bool {
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 =
@@ -403,7 +409,7 @@ fn visit_node(&mut self, node: Node<'tcx>) {
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 @@ fn visit_node(&mut self, node: Node<'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 @@ fn visit_node(&mut self, node: Node<'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 @@ fn visit_node(&mut self, node: Node<'tcx>) {
_ => {}
}
}
- intravisit::walk_impl_item(self, impl_item);
+ intravisit::walk_impl_item(self, impl_item)
}
- Node::ForeignItem(foreign_item) => {
- intravisit::walk_foreign_item(self, foreign_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 @@ fn check_impl_or_impl_item_live(&mut self, local_def_id: LocalDefId) -> bool {
}
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 @@ fn visit_variant_data(&mut self, def: &'tcx hir::VariantData<'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 @@ fn visit_expr(&mut self, expr: &'tcx hir::Expr<'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 @@ fn visit_pat(&mut self, pat: &'tcx hir::Pat<'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 @@ fn visit_pat_expr(&mut self, expr: &'tcx rustc_hir::PatExpr<'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 @@ fn visit_trait_ref(&mut self, t: &'tcx hir::TraitRef<'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 @@ fn is_live_code(&self, def_id: LocalDefId) -> bool {
}
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);
diff --git a/tests/crashes/125323.rs b/tests/crashes/125323.rs
deleted file mode 100644
index 180b7bb..0000000
--- a/tests/crashes/125323.rs
+++ /dev/null
@@ -1,6 +0,0 @@
-//@ known-bug: rust-lang/rust#125323
-fn main() {
- for _ in 0..0 {
- [(); loop {}];
- }
-}
diff --git a/tests/ui/consts/do-not-ice-long-constant-evaluation-in-for-loop.rs b/tests/ui/consts/do-not-ice-long-constant-evaluation-in-for-loop.rs
new file mode 100644
index 0000000..465b54e
--- /dev/null
+++ b/tests/ui/consts/do-not-ice-long-constant-evaluation-in-for-loop.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();
+}
diff --git a/tests/ui/consts/do-not-ice-long-constant-evaluation-in-for-loop.stderr b/tests/ui/consts/do-not-ice-long-constant-evaluation-in-for-loop.stderr
new file mode 100644
index 0000000..32a1846
--- /dev/null
+++ b/tests/ui/consts/do-not-ice-long-constant-evaluation-in-for-loop.stderr
@@ -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
+
