New scoping rules for safe destruction; not ready to land #20539
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
…on errors during regionck.
…er::SubregionOrigin`. Must rename breadcrumbs, or at least improve its documentation.
…ope`. This version has a debug statement that I should remove. also adds lifetime param to mem_categorization::scope to get repr working.
…_expr. includes still more debug instrumentation, as well as a span_note in the case that use to return prematurely.
…solutionError is pushed.
dropck handling of relationship of a closure to its enclosed block.
parent-scope of closure's block. This is (hopefully) temporary fix for boot-strapping pcwalton's old 8861 work, since my goal is to eventually get rid of `ReFunction` entirely.
includes debug output noting that we could have produced ReFunction here.
CodeExtent::Misc differentiation.
Namely, `BlockS` used to carry an `fcx: &'blk FunctionContext`, while the `FunctionContext` carries a reference to the arena that holds the blocks, and thus there was a chicken/egg problem where we are attempting to assign the lifetime `'blk` to both the `FunctionContext` and to the arena's blocks, which does not work under the new lifetime rules for `Drop`, since there has to be some order in which the two are torn down. To work around this, I removed the `fcx` from the `BlockS`, and instead turn the `Block` type (which was formerly `&'blk BlockS`) into a tuple carrying both the pointer to the `BlockS` as well as the `fcx`.
Accomodate new lifetime restrictions by ensuring `type_arena` outlives `ast_map`.
Accomodate new lifetime restrictions by ensuring `type_arena` outlives `ast_map`.
(I often run `compiletest` by hand by cut-and-pasting from what `make` runs, but then I need to tweak it (cut out options) and its useful to be told when I have removed an option that is actually required, such as `--android-cross-path=path`.)
… branch. This needs investigation: What did pnkfelix do that caused this to change?
the breadcrumbs until end of traversal.
In particular, ensure that both of the thread objects created for nlist and clist both outlive nlist and clist themselves.
It is very unfortunate that the destruction scopes are not printed in some distinguished way here. It is also unfortunate that the graphviz tests are so hard to maintain -- it would probably be better to just have a flowgraph graphviz mode that does not include the exiting-scope edge labels, and then only test in that mode.
…that borrows it. (part of fallout from new destructor lifetime rules.)
|
r? @huonw (rust_highfive has picked a reviewer for you, use r? to override) |
|
@nikomatsakis just a reminder: I am hoping to get feedback on the parts that would actually land in the end. If it is too much trouble to filter out the wheat from the chaff here with your eyes, just say so, and I can either try to provide you with some direction as to which commits to focus on, or I might just spend the time to make a slightly more focused PR. |
|
Just wanted to make sure #8861 (comment) gets seen here, since I think this is a safe generalization that will make the transition less painful (specifically: I think equal lifetime references can only cause an issue with explicit |
Though arguably it might be better to refactor the test to avoid having to make that method public.
|
@nikomatsakis (the diff may be a bit more manageable now. this seems likely to pass |
|
closing; the new version is PR #21022 |
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.
Not ready to land, in any way.
This is a resurrection and heavy revision/expansion of a PR that pcwalton did to resolve #8861.
I am just putting this up to get some early review feedback from Niko.
I put in some commits today that address some necessary fallout that I had been missing. I had previously overlooked a number of tests that were not being run because the test suite was getting hung up on a case of exponential time blowup (I think) in the dropck code for which I put in a quick work-around. (I think pcwalton’s original branch was the one that introduced the “breadcrumbs” field to avoid non-termination; the workaround further exploits the breadcrumbs structure to try to avoid unnecessary retraversals. It seems to have done the trick.
There may still be yet more fallout to be addressed; pnkfelix@31cec27 is an example of the kind of fix that I hope will be the most that will be required on a case-by-case basis. (The more extreme kind of fix is illustrated by pnkfelix@5dd63c1 but I really hope there are not many occurrences of that sort of thing in the tests.)
Also obviously the
src/libst2/stuff was just a skeletal clone of libstd to help me track down some issues when I was getting this bootstrapped; it obviously will be deleted in the real PR.