Use the impls module to import pre-existing dataflow analyses
#71893
+20
−25
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors r+ |
|
📌 Commit 095d1fd has been approved by |
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 6, 2020
Rollup of 7 pull requests Successful merges: - rust-lang#71269 (Define UB in float-to-int casts to saturate) - rust-lang#71591 (use new interface to create threads on HermitCore) - rust-lang#71819 (x.py: Give a more helpful error message if curl isn't installed) - rust-lang#71893 (Use the `impls` module to import pre-existing dataflow analyses) - rust-lang#71929 (Use -fvisibility=hidden for libunwind) - rust-lang#71937 (Ignore SGX on a few ui tests) - rust-lang#71944 (Add comment for `Ord` implementation for array) Failed merges: r? @ghost
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 22, 2020
…e-analysis, r=tmandry Clean up logic around live locals in generator analysis Resolves rust-lang#69902. Requires rust-lang#71893. I've found it difficult to make changes in the logic around live locals in `generator/transform.rs`. It uses a custom dataflow analysis, `MaybeRequiresStorage`, that AFAICT computes whether a local is either initialized or borrowed. That analysis is using `before` effects, which we should try to avoid if possible because they are harder to reason about than ones only using the unprefixed effects. @pnkfelix has suggested removing "before" effects entirely to simplify the dataflow framework, which I might pursue someday. This PR replaces `MaybeRequiresStorage` with a combination of the existing `MaybeBorrowedLocals` and a new `MaybeInitializedLocals`. `MaybeInitializedLocals` is just `MaybeInitializedPlaces` with a coarser resolution: it works on whole locals instead of move paths. As a result, I was able to simplify the logic in `compute_storage_conflicts` and `locals_live_across_suspend_points`. This is not exactly equivalent to the old logic; some generators are now smaller than before. I believe this was because the old logic was too conservative, but I'm not as familiar with the constraints as the original implementers were, so I could be wrong. For example, I don't see a reason the size of the `mixed_sizes` future couldn't be 5K. It went from 7K to 6K in this PR. r? @jonas-schievink @tmandry
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, existing analyses live in the same module as the traits and types used to define new dataflow analyses. This muddles the documentation for the
dataflowmodule. After this PR,dataflow::implswill refer to concrete dataflow analyses, anddataflowto the generic interface.