Major revision to the dropck_legal_cycles test. #28929
Merged
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
1. Added big comment block explaining the test framework. 2. Added tests exericising Rc and Arc. This was inspired by a comment from eefriedman on PR rust-lang#28861. 3. Made the cycle-detection not issue false-positives on acyclic dags. Doing this efficiently required revising the framework; instead of visiting all children (i.e. doing a traversal), now each test is responsible for supplying the path that will act as a witness to the cycle. Luckily for me, all of the pre-existing tests worked with a trivial path built from "always tke your first left", but new tests I added did require other input paths (i.e., "first turn right, then left". (The path representation is a bit-string and its branches are n-ary, not word phrases and binary branches as you might think from the outline above.)
|
(rust_highfive has picked a reviewer for you, use r? to override) |
pnkfelix
added a commit
to pnkfelix/rust
that referenced
this pull request
Oct 9, 2015
This was proven necessary after I added `Rc` and `Arc` to the rpass test `dropck_legal_cycles.rs`; see PR rust-lang#28929.
|
⌛ Testing commit 098a7a0 with merge e3376d8... |
bors
added a commit
that referenced
this pull request
Oct 9, 2015
Major revision to the dropck_legal_cycles test. 1. Added big comment block explaining the test framework. 2. Added tests exericising Rc and Arc. This was inspired by a comment from eefriedman on PR #28861. 3. Made the cycle-detection not issue false-positives on acyclic dags. Doing this efficiently required revising the framework; instead of visiting all children (i.e. doing a traversal), now each test is responsible for supplying the path that will act as a witness to the cycle. Luckily for me, all of the pre-existing tests worked with a trivial path built from "always tke your first left", but new tests I added did require other input paths (i.e., "first turn right, then left". (The path representation is a bit-string and its branches are n-ary, not word phrases and binary branches as you might think from the outline above.) cc PR #27185
pnkfelix
added a commit
to pnkfelix/rust
that referenced
this pull request
Oct 14, 2015
This was proven necessary after I added `Rc` and `Arc` to the rpass test `dropck_legal_cycles.rs`; see PR rust-lang#28929.
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this pull request
May 25, 2021
Weak's type parameter may dangle on drop Way back in rust-lang@34076bc, #\[may_dangle\] was added to Rc\<T\> and Arc\<T\>'s Drop impls. That appears to have been because a test added in rust-lang#28929 used Arc and Rc with dangling references at drop time. However, Weak was not covered by that test, and therefore no #\[may_dangle\] was forced to be added at the time. As far as dropping, Weak has *even less need* to interact with the T than Rc and Arc do. Roughly speaking #\[may_dangle\] describes generic parameters that the outer type's Drop impl does not interact with except by possibly dropping them; no other interaction (such as trait method calls on the generic type) is permissible. It's clear this applies to Rc's and Arc's drop impl, which sometimes drop T but otherwise do not interact with one. It applies *even more* to Weak. Dropping a Weak cannot ever cause T's drop impl to run. Either there are strong references still in existence, in which case better not drop the T. Or there are no strong references still in existence, in which case the T would already have been dropped previously by the drop of the last strong count.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 26, 2021
Weak's type parameter may dangle on drop Way back in rust-lang@34076bc, #\[may_dangle\] was added to Rc\<T\> and Arc\<T\>'s Drop impls. That appears to have been because a test added in rust-lang#28929 used Arc and Rc with dangling references at drop time. However, Weak was not covered by that test, and therefore no #\[may_dangle\] was forced to be added at the time. As far as dropping, Weak has *even less need* to interact with the T than Rc and Arc do. Roughly speaking #\[may_dangle\] describes generic parameters that the outer type's Drop impl does not interact with except by possibly dropping them; no other interaction (such as trait method calls on the generic type) is permissible. It's clear this applies to Rc's and Arc's drop impl, which sometimes drop T but otherwise do not interact with one. It applies *even more* to Weak. Dropping a Weak cannot ever cause T's drop impl to run. Either there are strong references still in existence, in which case better not drop the T. Or there are no strong references still in existence, in which case the T would already have been dropped previously by the drop of the last strong count.
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.
Major revision to the dropck_legal_cycles test.
Added big comment block explaining the test framework.
Added tests exericising Rc and Arc. This was inspired by a comment
from eefriedman on PR implement RFC 1238: nonparametric dropck. #28861.
Made the cycle-detection not issue false-positives on acyclic dags.
Doing this efficiently required revising the framework; instead of
visiting all children (i.e. doing a traversal), now each test is
responsible for supplying the path that will act as a witness to
the cycle.
Luckily for me, all of the pre-existing tests worked with a trivial
path built from "always tke your first left", but new tests I added
did require other input paths (i.e., "first turn right, then left".
(The path representation is a bit-string and its branches are
n-ary, not word phrases and binary branches as you might think
from the outline above.)
cc PR #27185