Skip to content

Commit 2e576ab

Browse files
committed
treat an error taint from const eval lint in late_lint and check_mod_deathness
error from const eval lint causes ICE at check_pat in late_lint, because the function expects the typeck result isn't tainted by error but it is. To avoid the ICE, check_pat returns earlier if the typeck_result is tainted. check_mod_deathness also has an issue from the same reason. visit_body for making live symbols expects the typeck result has no error. So this commit adds a check in visit_nested_body to avoid the ICE. However, if visit_nested_body just returns without doing anything, all codes with the error are marked as dead, because live_symbols is empty. To avoid this side effect, visit_nested_body and other visit_* functions in MarkSymbolVistior should return appropriate error. If a function returns ControlFlow::Break, live_symbols_and_ignore_derived_traits returns earlier with error, then check_mod_deathness, the caller of the function returns earlier without pushing everything into dead_codes.
1 parent 523d399 commit 2e576ab

File tree

6 files changed

+114
-52
lines changed

6 files changed

+114
-52
lines changed

compiler/rustc_lint/src/builtin.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,10 @@ declare_lint_pass!(NonShorthandFieldPatterns => [NON_SHORTHAND_FIELD_PATTERNS]);
153153

154154
impl<'tcx> LateLintPass<'tcx> for NonShorthandFieldPatterns {
155155
fn check_pat(&mut self, cx: &LateContext<'_>, pat: &hir::Pat<'_>) {
156-
if let PatKind::Struct(ref qpath, field_pats, _) = pat.kind {
156+
// The result shouldn't be tainted, otherwise it will cause ICE.
157+
if let PatKind::Struct(ref qpath, field_pats, _) = pat.kind
158+
&& cx.typeck_results().tainted_by_errors.is_none()
159+
{
157160
let variant = cx
158161
.typeck_results()
159162
.pat_ty(pat)

compiler/rustc_middle/src/query/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,10 +1200,10 @@ rustc_queries! {
12001200
/// Return the live symbols in the crate for dead code check.
12011201
///
12021202
/// The second return value maps from ADTs to ignored derived traits (e.g. Debug and Clone).
1203-
query live_symbols_and_ignored_derived_traits(_: ()) -> &'tcx (
1203+
query live_symbols_and_ignored_derived_traits(_: ()) -> &'tcx Result<(
12041204
LocalDefIdSet,
12051205
LocalDefIdMap<FxIndexSet<DefId>>,
1206-
) {
1206+
), ErrorGuaranteed> {
12071207
arena_cache
12081208
desc { "finding live symbols in crate" }
12091209
}

compiler/rustc_passes/src/dead.rs

Lines changed: 78 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,12 @@
44
// is dead.
55

66
use std::mem;
7+
use std::ops::ControlFlow;
78

89
use hir::def_id::{LocalDefIdMap, LocalDefIdSet};
910
use rustc_abi::FieldIdx;
1011
use rustc_data_structures::fx::FxIndexSet;
11-
use rustc_errors::MultiSpan;
12+
use rustc_errors::{ErrorGuaranteed, MultiSpan};
1213
use rustc_hir::def::{CtorOf, DefKind, Res};
1314
use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId};
1415
use rustc_hir::intravisit::{self, Visitor};
@@ -178,12 +179,12 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
178179
.iter()
179180
.any(|adj| matches!(adj.kind, ty::adjustment::Adjust::Deref(_)))
180181
{
181-
self.visit_expr(expr);
182+
let _ = self.visit_expr(expr);
182183
} else if let hir::ExprKind::Field(base, ..) = expr.kind {
183184
// Ignore write to field
184185
self.handle_assign(base);
185186
} else {
186-
self.visit_expr(expr);
187+
let _ = self.visit_expr(expr);
187188
}
188189
}
189190

@@ -318,7 +319,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
318319
}
319320
}
320321

321-
fn mark_live_symbols(&mut self) {
322+
fn mark_live_symbols(&mut self) -> <MarkSymbolVisitor<'tcx> as Visitor<'tcx>>::Result {
322323
while let Some(work) = self.worklist.pop() {
323324
let (mut id, comes_from_allow_expect) = work;
324325

@@ -366,8 +367,13 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
366367
continue;
367368
}
368369

369-
self.visit_node(self.tcx.hir_node_by_def_id(id));
370+
let visit_result = self.visit_node(self.tcx.hir_node_by_def_id(id));
371+
if visit_result.is_break() {
372+
return visit_result;
373+
}
370374
}
375+
376+
ControlFlow::Continue(())
371377
}
372378

373379
/// Automatically generated items marked with `rustc_trivial_field_reads`
@@ -395,19 +401,22 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
395401
false
396402
}
397403

398-
fn visit_node(&mut self, node: Node<'tcx>) {
404+
fn visit_node(
405+
&mut self,
406+
node: Node<'tcx>,
407+
) -> <MarkSymbolVisitor<'tcx> as Visitor<'tcx>>::Result {
399408
if let Node::ImplItem(hir::ImplItem { owner_id, .. }) = node
400409
&& self.should_ignore_item(owner_id.to_def_id())
401410
{
402-
return;
411+
return ControlFlow::Continue(());
403412
}
404413

405414
let unconditionally_treated_fields_as_live =
406415
self.repr_unconditionally_treats_fields_as_live;
407416
let had_repr_simd = self.repr_has_repr_simd;
408417
self.repr_unconditionally_treats_fields_as_live = false;
409418
self.repr_has_repr_simd = false;
410-
match node {
419+
let walk_result = match node {
411420
Node::Item(item) => match item.kind {
412421
hir::ItemKind::Struct(..) | hir::ItemKind::Union(..) => {
413422
let def = self.tcx.adt_def(item.owner_id);
@@ -417,7 +426,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
417426

418427
intravisit::walk_item(self, item)
419428
}
420-
hir::ItemKind::ForeignMod { .. } => {}
429+
hir::ItemKind::ForeignMod { .. } => ControlFlow::Continue(()),
421430
hir::ItemKind::Trait(.., trait_item_refs) => {
422431
// mark assoc ty live if the trait is live
423432
for trait_item in trait_item_refs {
@@ -435,7 +444,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
435444
if let Some(trait_id) = self.tcx.trait_of_assoc(trait_item_id) {
436445
self.check_def_id(trait_id);
437446
}
438-
intravisit::walk_trait_item(self, trait_item);
447+
intravisit::walk_trait_item(self, trait_item)
439448
}
440449
Node::ImplItem(impl_item) => {
441450
let item = self.tcx.local_parent(impl_item.owner_id.def_id);
@@ -456,16 +465,16 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
456465
_ => {}
457466
}
458467
}
459-
intravisit::walk_impl_item(self, impl_item);
460-
}
461-
Node::ForeignItem(foreign_item) => {
462-
intravisit::walk_foreign_item(self, foreign_item);
468+
intravisit::walk_impl_item(self, impl_item)
463469
}
470+
Node::ForeignItem(foreign_item) => intravisit::walk_foreign_item(self, foreign_item),
464471
Node::OpaqueTy(opaq) => intravisit::walk_opaque_ty(self, opaq),
465-
_ => {}
466-
}
472+
_ => ControlFlow::Continue(()),
473+
};
467474
self.repr_has_repr_simd = had_repr_simd;
468475
self.repr_unconditionally_treats_fields_as_live = unconditionally_treated_fields_as_live;
476+
477+
walk_result
469478
}
470479

471480
fn mark_as_used_if_union(&mut self, adt: ty::AdtDef<'tcx>, fields: &[hir::ExprField<'_>]) {
@@ -520,15 +529,25 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
520529
}
521530

522531
impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
523-
fn visit_nested_body(&mut self, body: hir::BodyId) {
524-
let old_maybe_typeck_results =
525-
self.maybe_typeck_results.replace(self.tcx.typeck_body(body));
532+
type Result = ControlFlow<ErrorGuaranteed>;
533+
534+
fn visit_nested_body(&mut self, body: hir::BodyId) -> Self::Result {
535+
let typeck_results = self.tcx.typeck_body(body);
536+
537+
// The result shouldn't be tainted, otherwise it will cause ICE.
538+
if let Some(guar) = typeck_results.tainted_by_errors {
539+
return ControlFlow::Break(guar);
540+
}
541+
542+
let old_maybe_typeck_results = self.maybe_typeck_results.replace(typeck_results);
526543
let body = self.tcx.hir_body(body);
527-
self.visit_body(body);
544+
let result = self.visit_body(body);
528545
self.maybe_typeck_results = old_maybe_typeck_results;
546+
547+
result
529548
}
530549

531-
fn visit_variant_data(&mut self, def: &'tcx hir::VariantData<'tcx>) {
550+
fn visit_variant_data(&mut self, def: &'tcx hir::VariantData<'tcx>) -> Self::Result {
532551
let tcx = self.tcx;
533552
let unconditionally_treat_fields_as_live = self.repr_unconditionally_treats_fields_as_live;
534553
let has_repr_simd = self.repr_has_repr_simd;
@@ -545,10 +564,10 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
545564
});
546565
self.live_symbols.extend(live_fields);
547566

548-
intravisit::walk_struct_def(self, def);
567+
intravisit::walk_struct_def(self, def)
549568
}
550569

551-
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
570+
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) -> Self::Result {
552571
match expr.kind {
553572
hir::ExprKind::Path(ref qpath @ QPath::TypeRelative(..)) => {
554573
let res = self.typeck_results().qpath_res(qpath, expr.hir_id);
@@ -584,20 +603,22 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
584603
_ => (),
585604
}
586605

587-
intravisit::walk_expr(self, expr);
606+
intravisit::walk_expr(self, expr)
588607
}
589608

590-
fn visit_arm(&mut self, arm: &'tcx hir::Arm<'tcx>) {
609+
fn visit_arm(&mut self, arm: &'tcx hir::Arm<'tcx>) -> Self::Result {
591610
// Inside the body, ignore constructions of variants
592611
// necessary for the pattern to match. Those construction sites
593612
// can't be reached unless the variant is constructed elsewhere.
594613
let len = self.ignore_variant_stack.len();
595614
self.ignore_variant_stack.extend(arm.pat.necessary_variants());
596-
intravisit::walk_arm(self, arm);
615+
let result = intravisit::walk_arm(self, arm);
597616
self.ignore_variant_stack.truncate(len);
617+
618+
result
598619
}
599620

600-
fn visit_pat(&mut self, pat: &'tcx hir::Pat<'tcx>) {
621+
fn visit_pat(&mut self, pat: &'tcx hir::Pat<'tcx>) -> Self::Result {
601622
self.in_pat = true;
602623
match pat.kind {
603624
PatKind::Struct(ref path, fields, _) => {
@@ -611,11 +632,13 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
611632
_ => (),
612633
}
613634

614-
intravisit::walk_pat(self, pat);
635+
let result = intravisit::walk_pat(self, pat);
615636
self.in_pat = false;
637+
638+
result
616639
}
617640

618-
fn visit_pat_expr(&mut self, expr: &'tcx rustc_hir::PatExpr<'tcx>) {
641+
fn visit_pat_expr(&mut self, expr: &'tcx rustc_hir::PatExpr<'tcx>) -> Self::Result {
619642
match &expr.kind {
620643
rustc_hir::PatExprKind::Path(qpath) => {
621644
// mark the type of variant live when meeting E::V in expr
@@ -628,37 +651,41 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
628651
}
629652
_ => {}
630653
}
631-
intravisit::walk_pat_expr(self, expr);
654+
intravisit::walk_pat_expr(self, expr)
632655
}
633656

634-
fn visit_path(&mut self, path: &hir::Path<'tcx>, _: hir::HirId) {
657+
fn visit_path(&mut self, path: &hir::Path<'tcx>, _: hir::HirId) -> Self::Result {
635658
self.handle_res(path.res);
636-
intravisit::walk_path(self, path);
659+
intravisit::walk_path(self, path)
637660
}
638661

639-
fn visit_anon_const(&mut self, c: &'tcx hir::AnonConst) {
662+
fn visit_anon_const(&mut self, c: &'tcx hir::AnonConst) -> Self::Result {
640663
// When inline const blocks are used in pattern position, paths
641664
// referenced by it should be considered as used.
642665
let in_pat = mem::replace(&mut self.in_pat, false);
643666

644667
self.live_symbols.insert(c.def_id);
645-
intravisit::walk_anon_const(self, c);
668+
let result = intravisit::walk_anon_const(self, c);
646669

647670
self.in_pat = in_pat;
671+
672+
result
648673
}
649674

650-
fn visit_inline_const(&mut self, c: &'tcx hir::ConstBlock) {
675+
fn visit_inline_const(&mut self, c: &'tcx hir::ConstBlock) -> Self::Result {
651676
// When inline const blocks are used in pattern position, paths
652677
// referenced by it should be considered as used.
653678
let in_pat = mem::replace(&mut self.in_pat, false);
654679

655680
self.live_symbols.insert(c.def_id);
656-
intravisit::walk_inline_const(self, c);
681+
let result = intravisit::walk_inline_const(self, c);
657682

658683
self.in_pat = in_pat;
684+
685+
result
659686
}
660687

661-
fn visit_trait_ref(&mut self, t: &'tcx hir::TraitRef<'tcx>) {
688+
fn visit_trait_ref(&mut self, t: &'tcx hir::TraitRef<'tcx>) -> Self::Result {
662689
if let Some(trait_def_id) = t.path.res.opt_def_id()
663690
&& let Some(segment) = t.path.segments.last()
664691
&& let Some(args) = segment.args
@@ -680,7 +707,7 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
680707
}
681708
}
682709

683-
intravisit::walk_trait_ref(self, t);
710+
intravisit::walk_trait_ref(self, t)
684711
}
685712
}
686713

@@ -827,7 +854,7 @@ fn create_and_seed_worklist(
827854
fn live_symbols_and_ignored_derived_traits(
828855
tcx: TyCtxt<'_>,
829856
(): (),
830-
) -> (LocalDefIdSet, LocalDefIdMap<FxIndexSet<DefId>>) {
857+
) -> Result<(LocalDefIdSet, LocalDefIdMap<FxIndexSet<DefId>>), ErrorGuaranteed> {
831858
let (worklist, mut unsolved_items) = create_and_seed_worklist(tcx);
832859
let mut symbol_visitor = MarkSymbolVisitor {
833860
worklist,
@@ -841,7 +868,9 @@ fn live_symbols_and_ignored_derived_traits(
841868
ignore_variant_stack: vec![],
842869
ignored_derived_traits: Default::default(),
843870
};
844-
symbol_visitor.mark_live_symbols();
871+
if let ControlFlow::Break(guar) = symbol_visitor.mark_live_symbols() {
872+
return Err(guar);
873+
}
845874

846875
// We have marked the primary seeds as live. We now need to process unsolved items from traits
847876
// and trait impls: add them to the work list if the trait or the implemented type is live.
@@ -855,14 +884,16 @@ fn live_symbols_and_ignored_derived_traits(
855884
symbol_visitor
856885
.worklist
857886
.extend(items_to_check.drain(..).map(|id| (id, ComesFromAllowExpect::No)));
858-
symbol_visitor.mark_live_symbols();
887+
if let ControlFlow::Break(guar) = symbol_visitor.mark_live_symbols() {
888+
return Err(guar);
889+
}
859890

860891
items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| {
861892
symbol_visitor.check_impl_or_impl_item_live(local_def_id)
862893
}));
863894
}
864895

865-
(symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits)
896+
Ok((symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits))
866897
}
867898

868899
struct DeadItem {
@@ -1142,7 +1173,11 @@ impl<'tcx> DeadVisitor<'tcx> {
11421173
}
11431174

11441175
fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
1145-
let (live_symbols, ignored_derived_traits) = tcx.live_symbols_and_ignored_derived_traits(());
1176+
let live_symbols_result = tcx.live_symbols_and_ignored_derived_traits(());
1177+
if live_symbols_result.is_err() {
1178+
return;
1179+
}
1180+
let (live_symbols, ignored_derived_traits) = live_symbols_result.as_ref().unwrap();
11461181
let mut visitor = DeadVisitor { tcx, live_symbols, ignored_derived_traits };
11471182

11481183
let module_items = tcx.hir_module_items(module);

tests/crashes/125323.rs

Lines changed: 0 additions & 6 deletions
This file was deleted.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// The test confirms ICE-125323 is fixed.
2+
//
3+
// This warning makes sure the fix doesn't throw everything with errors to dead.
4+
#![warn(unused)]
5+
fn should_not_be_dead() {}
6+
7+
fn main() {
8+
for _ in 0..0 {
9+
[(); loop {}]; //~ ERROR constant evaluation is taking a long time
10+
}
11+
12+
should_not_be_dead();
13+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
error: constant evaluation is taking a long time
2+
--> $DIR/do-not-ice-long-constant-evaluation-in-for-loop.rs:9:14
3+
|
4+
LL | [(); loop {}];
5+
| ^^^^^^^
6+
|
7+
= note: this lint makes sure the compiler doesn't get stuck due to infinite loops in const eval.
8+
If your compilation actually takes a long time, you can safely allow the lint.
9+
help: the constant being evaluated
10+
--> $DIR/do-not-ice-long-constant-evaluation-in-for-loop.rs:9:14
11+
|
12+
LL | [(); loop {}];
13+
| ^^^^^^^
14+
= note: `#[deny(long_running_const_eval)]` on by default
15+
16+
error: aborting due to 1 previous error
17+

0 commit comments

Comments
 (0)