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

Leave promoteds untainted by errors when borrowck fails #111038

Merged
merged 1 commit into from
May 2, 2023

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Apr 30, 2023

Previously, when borrowck failed it would taint all promoteds within the MIR body. An attempt to evaluated the promoteds would subsequently fail with spurious "note: erroneous constant used". For example:

...
note: erroneous constant used
 --> tests/ui/borrowck/tainted-promoteds.rs:7:9
  |
7 |     a = &0 * &1 * &2 * &3;
  |         ^^

note: erroneous constant used
 --> tests/ui/borrowck/tainted-promoteds.rs:7:14
  |
7 |     a = &0 * &1 * &2 * &3;
  |              ^^

note: erroneous constant used
 --> tests/ui/borrowck/tainted-promoteds.rs:7:19
  |
7 |     a = &0 * &1 * &2 * &3;
  |                   ^^

note: erroneous constant used
 --> tests/ui/borrowck/tainted-promoteds.rs:7:24
  |
7 |     a = &0 * &1 * &2 * &3;
  |                        ^^

Borrowck failure doesn't indicate that there is anything wrong with promoteds. Leave them untainted.

Fixes #110856.

Previously, when borrowck failed it would taint all promoteds within the MIR
body. An attempt to evaluated the promoteds would subsequently fail with
spurious "note: erroneous constant used". For example:

```console
...
note: erroneous constant used
 --> tests/ui/borrowck/tainted-promoteds.rs:7:9
  |
7 |     a = &0 * &1 * &2 * &3;
  |         ^^

note: erroneous constant used
 --> tests/ui/borrowck/tainted-promoteds.rs:7:14
  |
7 |     a = &0 * &1 * &2 * &3;
  |              ^^

note: erroneous constant used
 --> tests/ui/borrowck/tainted-promoteds.rs:7:19
  |
7 |     a = &0 * &1 * &2 * &3;
  |                   ^^

note: erroneous constant used
 --> tests/ui/borrowck/tainted-promoteds.rs:7:24
  |
7 |     a = &0 * &1 * &2 * &3;
  |                        ^^
```

Borrowck failure doesn't indicate that there is anything wrong with
promoteds. Leave them untainted.
@rustbot
Copy link
Collaborator

rustbot commented Apr 30, 2023

r? @lcnr

(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 Apr 30, 2023
@rustbot
Copy link
Collaborator

rustbot commented Apr 30, 2023

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@compiler-errors
Copy link
Member

Yeah, I have no idea why I made the tainted_by_errors propagate into the promoted constant in #93691.

I don't really think there are ways of triggering a (meaningful) borrowck error on a promoted constant anyways, though happy to be disproven by an example...

I checked, it also gets rid of the "erroneous constant" note at the end of:

fn main() {
    println!("{}", {
        let x: &str;
        x
    });
}
error[E0381]: used binding `x` isn't initialized
 --> /home/gh-compiler-errors/test.rs:4:9
  |
3 |         let x: &str;
  |             - binding declared here but left uninitialized
4 |         x
  |         ^ `x` used here but it isn't initialized
  |
help: consider assigning a value
  |
3 |         let x: &str = todo!();
  |                     +++++++++

note: erroneous constant used
 --> /home/gh-compiler-errors/test.rs:2:14
  |
2 |     println!("{}", {
  |              ^^^^

r=me when green

@matthewjasper
Copy link
Contributor

There's the error in #71587, not sure if that counts as "meaningful" or not.

@compiler-errors
Copy link
Member

@matthewjasper: Sorry, by "meaningful", I guess I should've been more clear. What I mean is borrowck errors that are likely to cause spurious const-eval errors or ICEs. It doesn't seem like that's the case with the class of issues you fixed in #71587, though, and it doesn't seem like any other existing UI tests were affected by this change, so I consider this PR to be pretty low-risk. The only possible issue is that we might see are spurious const eval errors or ICEs from no longer tainting these promoteds, thus leading to them being evaluated...

@bors r+

@bors
Copy link
Contributor

bors commented May 1, 2023

📌 Commit c678acd has been approved by compiler-errors

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 May 1, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request May 1, 2023
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#109540 (std docs: edit `PathBuf::set_file_name` example)
 - rust-lang#110093 (Add 64-bit `time_t` support on 32-bit glibc Linux to `set_times`)
 - rust-lang#110987 (update wasi_clock_time_api ref.)
 - rust-lang#111038 (Leave promoteds untainted by errors when borrowck fails)
 - rust-lang#111042 (Add `#[no_coverage]` to the test harness's `fn main`)
 - rust-lang#111057 (Make sure the implementation of TcpStream::as_raw_fd is fully inlined)
 - rust-lang#111065 (Explicitly document how Send and Sync relate to references)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f835b13 into rust-lang:master May 2, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 2, 2023
@tmiasko tmiasko deleted the untainted-promoteds branch May 2, 2023 09:08
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.

erroneous note: erroneous constant used emitted for non-compiling format_args! usage
6 participants