Skip to content
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.

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

fix: return early when has tainted in mir-lint #115643

Merged
merged 1 commit into from
Sep 8, 2023

Conversation

bvanjoi
Copy link
Contributor

@bvanjoi bvanjoi commented Sep 7, 2023

Fixes #115203

a[..] is of indeterminate size, it had been reported error during borrow check, therefore we skip the mir lint process.

@rustbot
Copy link
Collaborator

rustbot commented Sep 7, 2023

r? @petrochenkov

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 7, 2023
@rustbot
Copy link
Collaborator

rustbot commented Sep 7, 2023

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

@@ -0,0 +1,11 @@
// compile-flags: --emit link
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am unsure about how to label this test header. Only run-fail can reproduce the ice of #115203, but it also requires the case to build successfully. Consequently, I have resorted to using --emit link.

@RalfJung
Copy link
Member

RalfJung commented Sep 7, 2023

Ugh, I thought we stopped running ConstProp on code that is tainted by type errors? Cc @oli-obk

@oli-obk
Copy link
Contributor

oli-obk commented Sep 7, 2023

Only if it actually gets tainted 😆

@oli-obk
Copy link
Contributor

oli-obk commented Sep 7, 2023

@bvanjoi if you can find out where the error is emitted in typeck, you can use https://doc.rust-lang.org/nightly/nightly-rustc/rustc_infer/infer/struct.InferCtxt.html#method.set_tainted_by_errors, which should also fix the issue

@rustbot
Copy link
Collaborator

rustbot commented Sep 7, 2023

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@bvanjoi
Copy link
Contributor Author

bvanjoi commented Sep 7, 2023

A taint already exists, which was appended by the buffer_error. Therefore, we only need to return early if a taint is present at the start of the run_lint function.

@@ -63,6 +63,10 @@ impl<'tcx> MirLint<'tcx> for ConstProp {
return;
}

if body.tainted_by_errors.is_some() {
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be checked at the top of the function, no need to do the check so late.

Now I wonder if this means we can get rid of some of the other ConstPropNonsense throws...

@bvanjoi bvanjoi changed the title fix: throw error when field is unsized fix: return early when has tainted in mir-lint Sep 8, 2023
@petrochenkov
Copy link
Contributor

r? @RalfJung

@rustbot rustbot assigned RalfJung and unassigned petrochenkov Sep 8, 2023
@RalfJung
Copy link
Member

RalfJung commented Sep 8, 2023

This LGTM, but oli understands this tainting business much better than I do...
r? @oli-obk

@rustbot rustbot assigned oli-obk and unassigned RalfJung Sep 8, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Sep 8, 2023

@bors r=RalfJung,oli-obk

We should probably skip all analyses after a body has been tainted. That may allow us to remove a bunch of delay span bugs in mir opts and the analysis passes and transforms, but that's for a separate PR

@bors
Copy link
Contributor

bors commented Sep 8, 2023

📌 Commit 967410c has been approved by RalfJung,oli-obk

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 8, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 8, 2023
…llaumeGomez

Rollup of 6 pull requests

Successful merges:

 - rust-lang#104299 (Clarify stability guarantee for lifetimes in enum discriminants)
 - rust-lang#115088 (Fix Step Skipping Caused by Using the `--exclude` Option)
 - rust-lang#115201 (rustdoc: list matching impls on type aliases)
 - rust-lang#115633 (Lint node for `PRIVATE_BOUNDS`/`PRIVATE_INTERFACES` is the item which names the private type)
 - rust-lang#115638 (`-Cllvm-args` usability improvement)
 - rust-lang#115643 (fix: return early when has tainted in mir-lint)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 60327bb into rust-lang:master Sep 8, 2023
11 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 8, 2023
Rollup merge of rust-lang#115643 - bvanjoi:fix-115203, r=RalfJung,oli-obk

fix: return early when has tainted in mir-lint

Fixes rust-lang#115203

`a[..]` is of indeterminate size, it had been reported error during borrow check, therefore we skip the mir lint process.
@rustbot rustbot added this to the 1.74.0 milestone Sep 8, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 13, 2023
fix: return early when has tainted in mir pass

Fixes rust-lang#115809

As in rust-lang#115643, `run_pass` is skipped if the body has tainted errors.

r? `@oli-obk`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 13, 2023
Rollup merge of rust-lang#115815 - bvanjoi:fix-115809, r=oli-obk

fix: return early when has tainted in mir pass

Fixes rust-lang#115809

As in rust-lang#115643, `run_pass` is skipped if the body has tainted errors.

r? `@oli-obk`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3d vector compiler bug
6 participants