Rollup of 17 pull requests #93138
Rollup of 17 pull requests #93138
Conversation
…ed implementations
This change adds the basic infrastructure for tracking drop ranges in generator interior analysis, which allows us to exclude dropped types from the generator type. Not yet complete, but many of the async/await and generator tests pass. The main missing piece is tracking branching control flow (e.g. around an `if` expression). The patch does include support, however, for multiple yields in th e same block. Issue rust-lang#57478
The main change needed to make this work is to do a pessimistic over- approximation for AssignOps. The existing ScopeTree analysis in region.rs works by doing both left to right and right to left order and then choosing the most conservative ordering. This behavior is needed because AssignOp's evaluation order depends on whether it is a primitive type or an overloaded operator, which runs as a method call. This change mimics the same behavior as region.rs in generator_interior.rs. Issue rust-lang#57478
This is needed to handle cases like `[a, b.await, c]`. `ExprUseVisitor` considers `a` to be consumed when it is passed to the array, but the array is not quite live yet at that point. This means we were missing the `a` value across the await point. Attributing drops to the parent expression means we do not consider the value consumed until the consuming expression has finished. Issue rust-lang#57478
This adds support for branching and merging control flow and uses this to correctly handle the case where a value is dropped in one branch of an if expression but not another. There are other cases we need to handle, which will come in follow up patches. Issue rust-lang#57478
Not currently working. Need to flow drop information.
All tests pass now! The issue was that we weren't handling all edges correctly, but now they are handled consistently. This includes code to dump a graphviz file for the CFG we built for drop tracking. Also removes old DropRanges tests.
1. Add test case for partial drops 2. Simplify code in `propagate_to_fixpoint` and remove most clones 3. Clean up PostOrderIndex creation
Splits drop_ranges into drop_ranges::record_consumed_borrow, drop_ranges::cfg_build, and drop_ranges::cfg_propagate. The top level drop_ranges module has an entry point that does all the coordination of the other three phases, using code original in generator_interior.
This cleans up the refactoring from the previous patch and cleans things up a bit. Each module has a clear entry point and everything else is private.
The refactoring mainly keeps the separation between the modules clearer. For example, process_deferred_edges function moved to cfg_build.rs since that is really part of building the CFG, not finding the fixpoint. Also, we use PostOrderId instead of usize in a lot more places now.
Also rearranges the existing arms to be more logical. For example, Break and Continue come closer to Loop now.
update comment for `ensure_monomorphic_enough` r? `@RalfJung`
…on-number, r=Mark-Simulacrum Add script to prevent point releases with same number as existing ones This will hopefully prevent what happened today with rust-lang#93110 and rust-lang#93121, where we built point release artifacts without changing version numbers, thus requiring another PR to change the version number. r? `@Mark-Simulacrum`
…ter, r=pietroalbini Backport the 1.58.1 release notes to master r? `@ghost`
rustbot
added
T-compiler
|
@bors r+ rollup=never p=17 |
|
📌 Commit dd16431 has been approved by |
bors
added
the
S-waiting-on-bors
|
This rollup includes an already merged PR #93038 |
|
☀️ Test successful - checks-actions |
Tested on commit rust-lang/rust@523be2e. Direct link to PR: <rust-lang/rust#93138> 💔 miri on windows: test-pass → test-fail (cc @oli-obk @eddyb @RalfJung). 💔 miri on linux: test-pass → test-fail (cc @oli-obk @eddyb @RalfJung).
|
Finished benchmarking commit (523be2e): comparison url. Summary: This change led to very large relevant mixed results 🤷 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression |
|
probably #91032 |
|
#93165 soft reverted #91032 which landed in this PR: the +879% perf results on |
|
The perf summary is interesting. These are instruction counts for running rustc on the test case, right? Not instruction counts for executing the compiled test program? If so it seems like for For |
This is correct. https://perf.rust-lang.org/ is focused solely on resources used by running Your hypothesis sounds plausible, though it is nonetheless a pretty amazing peformance change. Is it perhaps a sign that the benchmark will need revision/replacement? I suppose that depends on whether you think it still serves as a realistic model even after drop range tracking is enabled... |
|
The If we want to keep something closer to the old behavior, I think we can make it so the nest of functions returns something with a destructor. I don't know whether it makes sense to do this by modifying the existing benchmark (which would invalidate old perf data) or adding a very similar benchmark beside the current one. What do you think? |
Successful merges:
!to expressions that can be macro invocation #93061 (Only suggest adding!to expressions that can be macro invocation)let_chainsworks withif_let_guard#93086 (Add tests to ensure thatlet_chainsworks withif_let_guard)struct_fields invariant_tuple_struct.rs#93094 (src/test/rustdoc-json: Check forstruct_fields invariant_tuple_struct.rs)DefPathHashdoes not exist #93098 (Show a more informative panic message whenDefPathHashdoes not exist)ensure_monomorphic_enough#93114 (update comment forensure_monomorphic_enough)Failed merges:
r? @ghost
@rustbot modify labels: rollup
Create a similar rollup