Skip to content

Check tainted_by_error in LateLint #147876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+33 −12
Open

Check tainted_by_error in LateLint #147876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
88738c7
visit_nested_body for LateLint should return earlier if there is an e…
Shunpoco Oct 19, 2025
f61c361
rename a test which checks long constant evaluation causes dead codes
Shunpoco Oct 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions compiler/rustc_lint/src/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,7 @@ declare_lint_pass!(NonShorthandFieldPatterns => [NON_SHORTHAND_FIELD_PATTERNS]);

impl<'tcx> LateLintPass<'tcx> for NonShorthandFieldPatterns {
fn check_pat(&mut self, cx: &LateContext<'_>, pat: &hir::Pat<'_>) {
// 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()
{
if let PatKind::Struct(ref qpath, field_pats, _) = pat.kind {
let variant = cx
.typeck_results()
.pat_ty(pat)
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_lint/src/late.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,14 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas
}

fn visit_nested_body(&mut self, body_id: hir::BodyId) {
// The typeck_result shouldn't be tainted, otherwise it will cause ICE.
// If rustdoc is the caller of this function, we shouldn't run typeck_body here.
if !self.context.tcx.sess.opts.actually_rustdoc
&& self.context.tcx.typeck_body(body_id).tainted_by_errors.is_some()
Copy link
Contributor

@oli-obk oli-obk Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering this can be called unconditionally outside rustdoc, perform it further down where we do self.context.cached_typeck_results.set(None); and just always set it to Some unless in rustdoc mode or if tainted (keep returning in the tainted case of course).

The cache still needs to be an option as we also have lints outside of bodies

Copy link
Contributor Author

@Shunpoco Shunpoco Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks,

Considering this can be called unconditionally outside rustdoc, perform it further down where we do self.context.cached_typeck_results.set(None);

Do you mean I should call typeck_body in the block which cached_typeck_results.set(None) is called or I should call typeck_body other place where rustdoc doesn't use?

Copy link
Contributor

@oli-obk oli-obk Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can call it there, but still behind an !actually_rustdoc check, and then just overwrite the freshly set None with a Some of the data you just loaded.

At that point I think we can even change the method for obtaining the cached typeck results to just unwrap the option

Copy link
Contributor Author

@Shunpoco Shunpoco Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, let me take time to investigate. I think I need to understand how typeck_body works more.

we can even change the method for obtaining~

Let me clarify, does "the method" point typeck_body?

Copy link
Contributor

@oli-obk oli-obk Nov 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me clarify, does "the method" point typeck_body?

No I meant maybe_typeck_results, which currently has an or_else, but could now just remove the or_else and trust that it was already filled in

{
return;
}

let old_enclosing_body = self.context.enclosing_body.replace(body_id);
let old_cached_typeck_results = self.context.cached_typeck_results.get();

Expand Down
6 changes: 0 additions & 6 deletions tests/crashes/138361.rs
View file
Open in desktop

This file was deleted.

0 ...e-long-constant-evaluation-in-for-loop.rs → ...ng-constant-evaluation-cause-dead-code.rs
View file
Open in desktop
File renamed without changes.
4 changes: 2 additions & 2 deletions ...ng-constant-evaluation-in-for-loop.stderr → ...onstant-evaluation-cause-dead-code.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
error: constant evaluation is taking a long time
--> $DIR/do-not-ice-long-constant-evaluation-in-for-loop.rs:10:14
--> $DIR/long-constant-evaluation-cause-dead-code.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
--> $DIR/long-constant-evaluation-cause-dead-code.rs:10:14
|
LL | [(); loop {}];
| ^^^^^^^
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/consts/long-constant-evaluation-cause-ice-in-sty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// The test confirms ICE-138361 is fixed.
fn main() {
[0; loop{}]; //~ ERROR constant evaluation is taking a long time
std::mem::transmute(4)
}
17 changes: 17 additions & 0 deletions tests/ui/consts/long-constant-evaluation-cause-ice-in-sty.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error: constant evaluation is taking a long time
--> $DIR/long-constant-evaluation-cause-ice-in-sty.rs:3:7
|
LL | [0; 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/long-constant-evaluation-cause-ice-in-sty.rs:3:7
|
LL | [0; loop{}];
| ^^^^^^
= note: `#[deny(long_running_const_eval)]` on by default

error: aborting due to 1 previous error

Loading