rustc_ast_lowering: misc cleanup & rustc dep reductions
- The first two commits do some code simplification.
- The next three do some file splitting (getting `lib.rs` below the 3kloc tidy lint).
- The remaining commits reduce the number of `rustc::` imports. This works towards making lowering independent of the `rustc` crate.
r? @oli-obk cc @Zoxc
Because it caused major performance regressions in some cases.
That PR had five commits, two of which affected performance, and three
of which were refactorings. This change undoes the performance-affecting
changes, while keeping the refactorings in place.
Fixes#67454.
The callbacks have precisely two states: the default, and the one
present throughout almost all of the rustc run (the filled in value
which has access to TyCtxt).
We used to store this as a thread local, and reset it on each thread to
the non-default value. But this is somewhat wasteful, since there is no
reason to set it globally -- while the callbacks themselves access TLS,
they do not do so in a manner that fails in when we do not have TLS to
work with.
This reverts commit 3ed3b8bb7b100afecf7d5f52eafbb70fec27f537, reversing
changes made to 99b89533d4cdf7682ea4054ad0ee36c351d05df1.
We will reland a similar patch at a future date but for now we should get a nightly
released in a few hours with the parallel patch, so this should be
reverted to make sure that the next nightly is not parallel-enabled.
In particular, it has bugged me for some time that `process_cycles` is
currently located before `mark_still_waiting_nodes` despite being called
afterwards.
`NodeState` has two states, `Success` and `Done`, that are only used
within `ObligationForest` methods. This commit removes them, and renames
the existing `Waiting` state as `Success`.
We are left with three states: `Pending`, `Success`, and `Error`.
`Success` is augmented with a new `WaitingState`, which indicates when
(if ever) it was last waiting on one or more `Pending` nodes. This
notion of "when" requires adding a "process generation" to
`ObligationForest`; it is incremented on each call to
`process_obligtions`.
This commit is a performance win.
- Most of the benefit comes from `mark_as_waiting` (which the commit
renames as `mark_still_waiting_nodes`). This function used to do two
things: (a) change all `Waiting` nodes to `Success`, and (b) mark all
nodes that depend on a pending node as `Waiting`. In practice, many
nodes went from `Waiting` to `Success` and then immediately back to
`Waiting`. The use of generations lets us skip step (a).
- A smaller benefit comes from not having to change nodes to the `Done`
state in `process_cycles`.
In which we implement illegal subset relations errors using Polonius
This PR is the rustc side of implementing subset errors using Polonius. That is, in
```rust
fn foo<'a, 'b>(x: &'a u32, y: &'b u32) -> &'a u32 {
y
}
```
returning `y` requires that `'b: 'a` but we have no evidence of that, so this is an error. (Evidence that the relation holds could come from explicit bounds, or via implied bounds).
Polonius outputs one such error per CFG point where the free region's placeholder loan unexpectedly flowed into another free region. While all these CFG locations could be useful in diagnostics in the future, rustc does not do that (and the duplication is only partially handled in the rest of the errors/diagnostics infrastructure, e.g. duplicate suggestions will be shown by the "outlives suggestions" or some of the `#[rustc_*]` NLL/MIR debug dumps), so I deduplicated the errors.
(The ordering also matters, otherwise some of the elided lifetime naming would change behaviour).
I've blessed a couple of tests, where the output is currently suboptimal:
- the `hrtb-perfect-forwarding` tests mix subset errors with higher-ranked subtyping, however the plan is for chalk to eventually take care of some of this to generate polonius constraints (i.e. it's not polonius' job). Until that happens, polonius will not see the error that NLL sees.
- some other tests have errors and diagnostics specific to `'static`, I _believe_ this to be because of it being treated as more "special" than in polonius. I believe the output is not wrong, but could be better, and appears elsewhere (I feel we'll need to look at polonius' handling of `'static` at some point in the future, maybe to match a bit more what NLL does when it produces errors)
I'll create a tracking issue in the polonius repo to record these 2 points (and a general "we'll need to go over the blessed output" issue, much like we did for NLLs)
The last blessed test is because it's an improvement: in this case, more errors/suggestions were computed, instead of the existing code path where this case apparently stops at the first error.
The `Naive` variant in Polonius computes those errors, so this PR also switches the default variant to that, as we're also in the process of temporarily deactivating all other variants (which exist mostly for performance considerations) until we have completed more work on completeness and correctness, before focusing on efficiency once again.
While most of the correctness in this PR is hidden in the polonius compare-mode (which of course passes locally), I've added a couple of smoke-tests to the existing ones, so that we have some confidence that it works (and keeps working) until we're in a position where we can run them on CI.
As mentioned during yesterday's wg-polonius meeting, @nikomatsakis has already read through most of this PR (and which is matching what they thought needed to be done [during the recent Polonius sprint](https://hackmd.io/CGMNjt1hR_qYtsR9hgdGmw#Compiler-notes-on-generating-the-placeholder-loans-support)), but Matthew was hopefully going to review (again, not urgent), so:
r? @matthewjasper
(This updates to the latest `polonius-engine` release, and I'm not sure whether `Cargo.lock` updates can easily be rolled up, but apart from that: this changes little that's tested on CI, so seems safe-ish to rollup ?)
Update measureme crate to 0.5.0
This PR updates the `measureme` self-profiling crate to the latest release. Heads up, this version changes the trace file format, so the `summarize` tool on perf.rlo needs to be updated to 0.5 too.
r? @Mark-Simulacrum
cc @wesleywiser
Make `process_obligations()` greedier.
`process_obligations()` adds new nodes, but it does not process these
new nodes until the next time it is called.
This commit changes it so that it does process these new nodes within
the same call. This change reduces the number of calls to
`process_obligations()` required to complete processing, sometimes
giving significant speed-ups.
The change required some changes to tests.
- The output of `cycle-cache-err-60010.rs` is slightly different.
- The unit tests required extra cases to handle the earlier processing
of the added nodes. I mostly did these in the simplest possible way,
by making the added nodes be ignored, thus giving outcomes the same as
with the old behaviour. But I changed `success_in_grandchildren()`
more extensively so that some obligations are completed earlier than
they used to be.
r? @nikomatsakis
rustdoc: Don't panic when failing to write .lock file
It can be treated like any other unexpected IO error.
I couldn't think of a good way to add a test for this unfortunately.
r? @GuillaumeGomez
Avoid hashing the key twice in `get_query()`.
For a single-threaded parallel compiler, this reduces instruction counts
across several benchmarks, by up to 2.8%.
The commit also adds documentation about `Sharded`'s use of `FxHasher`.
r? @Zoxc
`process_obligations()` adds new nodes, but it does not process these
new nodes until the next time it is called.
This commit changes it so that it does process these new nodes within
the same call. This change reduces the number of calls to
`process_obligations()` required to complete processing, sometimes
giving significant speed-ups.
The change required some changes to tests.
- The output of `cycle-cache-err-60010.rs` is slightly different.
- The unit tests required extra cases to handle the earlier processing
of the added nodes. I mostly did these in the simplest possible way,
by making the added nodes be ignored, thus giving outcomes the same as
with the old behaviour. But I changed `success_in_grandchildren()`
more extensively so that some obligations are completed earlier than
they used to be.
The single dependency on queries (QueryName) can be fairly easily
abstracted via a trait and this further decouples Session from librustc
(the primary goal).
For a single-threaded parallel compiler, this reduces instruction counts
across several benchmarks, by up to 2.8%.
The commit also adds documentation about `Sharded`'s use of `FxHasher`.
Add lint and tests for unnecessary parens around types
This is my first contribution to the Rust project, so I apologize if I'm not doing things the right way.
The PR fixes#64169. It adds a lint and tests for unnecessary parentheses around types. I've run `tidy` and `rustfmt` — I'm not totally sure it worked right, though — and I've tried to follow the instructions linked in the readme.
I tried to think through all the variants of `ast::TyKind` to find exceptions to this lint, and I could only find the one mentioned in the original issue, which concerns types with `dyn`. I'm not a Rust expert, thought, so I may well be missing something.
There's also a problem with getting this to build. The new lint catches several things in the, e.g., `core`. Because `x.py` seems to build with an equivalent of `-Werror`, what would have been warnings cause the build to break. I got it to build and the tests to pass with `--warnings warn` on my `x.py build` and `x.py test` commands.
Eliminate `intersect_opt`.
Its fourth argument is always `Some(pred)`, so the pattern matching is
unnecessary. This commit inlines and removes it.
r? @nikomatsakis
