owned this note
owned this note
Published
Linked with GitHub
Hello Matthew :)
===
Thanks in advance for your time.
I'm currently going through the ui test failures under `-Z polonius`, which are basically at the same point as when you last re-enabled rustc's polonius support in [#54468](https://github.com/rust-lang/rust/pull/54468).
I'm taking notes in [this document](https://hackmd.io/CjYB0fs4Q9CweyeTdKWyEg?view) and had a couple questions on some of these failures, different reductions, etc.
### 1. Casts ?
I'll start off with an easy one: I was looking at [borrowck/two-phase-surprise-no-conflict.rs](https://github.com/rust-lang/rust/blob/8fa44c061b7d2db63eed229810f34f1e13633eea/src/test/ui/borrowck/two-phase-surprise-no-conflict.rs) and the [NLL output](https://raw.githubusercontent.com/rust-lang/rust/8fa44c061b7d2db63eed229810f34f1e13633eea/src/test/ui/borrowck/two-phase-surprise-no-conflict.stderr) mentions casts:
```rust
error[E0502]: cannot borrow `*reg` as mutable because it is also borrowed as immutable
--> $DIR/two-phase-surprise-no-conflict.rs:141:5
|
LL | fn register_plugins<'a>(mk_reg: impl Fn() -> &'a mut Registry<'a>) {
| -- lifetime `'a` defined here
...
LL | reg.register_univ(Box::new(CapturePass::new(®.sess_mut)));
| ^^^^^^^^^^^^^^^^^^-----------------------------------------^
| | | |
| | | immutable borrow occurs here
| | cast requires that `reg.sess_mut` is borrowed for `'a`
| mutable borrow occurs here
```
does this diagnostics' wording seem ok to you ?
A: *It's OK for now, ideally we would instead have "later used here for call" as the message (pointing as `register_univ`), but an optimization that we're doing suppresses the required information for that error.*
### 2. Overflow when generating NLL facts
I haven't created an issue for [this error](https://hackmd.io/CjYB0fs4Q9CweyeTdKWyEg?both#24-uiissuesissue-38591rs---output-from-Polonius) yet. Do you have an educated guess of whether this would be easy to fix ? I have not yet tried to debug this, and just saw that it's likely in, or triggered by, the fact generation rather than in Polonius or downstream.
A: *The compiler will often overflow for ADTs with infinitely many transitive field types. This is either fairly easy or very hard to fix, but knowing which requires investigation.*
### 3. Loops
There are some weird errors and a lot of them involve loops.
My ultimate goal is to find out whether they are
- Polonius bugs, or the known imprecision w.r.t. subset propagation (and these tests should have never worked, and maybe they never did work)
- a bug in rustc's integration (e.g. in fact generation) either because of some known missing feature or deactivation (like some of the optimizations which are disabled when using Polonius) or regressions due to desugaring or MIR changes, and so on
So I wanted to check if you saw anything stand out from these cases as immediately obvious (without looking at them in any detail, don't waste your time on this) or if you had a piece of advice to point me at possible interesting areas to trace, debug, and investigate further.
#### 3.1 — The one I looked at most: [a loop from `rand`](https://hackmd.io/CjYB0fs4Q9CweyeTdKWyEg?both#20-the-rand-crate-difference-not-a-rustc-test-but-something-that-needs-to-be-done-anyway-)
This one is interesting because we can basically reduce it to [a simpler (`reduced_loop` fn)](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=03d308bdf21f67ba12a37a980874d1cb) version of issue [#47680](https://github.com/rust-lang/polonius/blob/master/inputs/issue-47680/issue-47680.rs). This resulting loop is accepted by NLL and rejected by Polonius.
I wonder if anything is either missing from the facts, or if there's a bug in Polonius here, and reducing the facts à la bugpoint, makes it look like a legit error so that doesn't tell us much (either a missing fact, or missing datalog rules to take it into account, or a wrong reduction ...).
Polonius finds 2 errors in this piece of code:
- bw2 being invalidated at `Start(bb3[3])` while the loan is live
- bw2 being invalidated at `Start(bb4[2])` while the loan is live
Some provenance information about the first error:
- bw2 is required in '7 at `Start(bb3[3])` and the region is live.
- At the predecessor `Mid(bb3[2])`, bw2 is required in '5, and '5 outlives '7.
- bw2 is required in '5 from the start of the loop `Start(bb2[0])`, and IIUC propagated from `Mid(bb4[2])` which creates the loan in '4, and '4 outlives '5.
(WIP provenance information about the second error:
- bw2 is required in '6 at `Start(bb4[2])` and the region is live
- ...)
Is the lack of any `killed` facts about bw2 -- or that there are no `killed` facts at `bb3[2]` either -- suspicious or expected ? Maybe it's more of Polonius' job to compute and propagate this information by itself (maybe to avoid a situation where rustc would have to do a polonius-like computation just to emit the polonius facts).
I will keep looking at this one since it looks deceptively simple for a case which doesn't work.
A: *This is an issue with fact generation. We only generate killed facts for `StatementKind::Assign`, but in the current implementation we also generate them for `StorageDead` and some `InlineAsm` outputs. We also don't handle assignments to projections correctly in fact generation.*
#### 3.2 — [borrowck/borrowck-lend-flow-loop.rs](https://hackmd.io/CjYB0fs4Q9CweyeTdKWyEg?both#2-borrowckborrowck-lend-flow-looprs-%E2%80%94-outputs-from-NLL-Polonius-diff)
Here, Polonius finds 2 additional errors, in `loop_break_pops_scopes` and `loop_loop_pops_scopes`.
The errors are when trying to consume the reference produced during the loop iteration.
The message mentions "previous iteration of loop" but `r` is created immediately before, so I'm not sure if the diagnostics is to be trusted.
#### 3.3 — [span/regions-escape-loop-via-vec.rs](https://hackmd.io/CjYB0fs4Q9CweyeTdKWyEg?both#18-spanregions-escape-loop-via-vecrs-%E2%80%94-outputs-from-NLL-Polonius-diff)
Here, Polonius finds one additional error, weirdly about `z` being borrowed in `let mut z = x;`.
---
I haven't traced the last 2 in polonius-engine itself yet. As mentioned before it's just to see if you see anything unusual stand out here (don't waste your time on this).
Thanks a lot in advance :)