-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expand dynamic drop tests for cases in #47949 #60729
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
c62bf96
to
85caf26
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, r=me, but can we tweak the comments?
let q = loop { | ||
a.alloc(); | ||
let r = a.alloc(); | ||
break a.alloc_leaked(exceptions); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to have a FIXME here too -- something like
// FIXME(#XXX) we should not be leaking this, but we are
// Creates a `Ptr<'_>` and checks that the allocated value is leaked if the | ||
// `failing_op` is in the list of exception. | ||
// | ||
// FIXME (#47949) We shouldn't need this, but we are sometimes leaking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason, I found this kind of confusing. Maybe something like this:
FIXME(#47949) -- Each use of this indicates a bug: basically, a value that winds up getting leaked but shouldn't be.
r=me -- @matthewjasper did you want to "FIXUP" that commit, thogh? |
49304b7
to
6a9e68a
Compare
@bors r=nikomatsakis |
📌 Commit 6a9e68a has been approved by |
…komatsakis Expand dynamic drop tests for cases in rust-lang#47949 Adds test cases for rust-lang#47949
Rollup of 8 pull requests Successful merges: - #60729 (Expand dynamic drop tests for cases in #47949) - #61263 (Don't generate div inside header (h4/h3/h...) elements) - #61364 (Stabilize reverse_bits feature) - #61375 (Make "panic did not include expected string" message consistent) - #61387 (Remove ty::BrFresh and RegionConstraintCollector::new_bound) - #61389 (Remove GlobalArenas and use Arena instead) - #61391 (Doc comment fixes for `rustc::mir::interpret::InterpretCx`) - #61403 (Remove unnecessary `-Z continue-parse-after-error` from tests) Failed merges: r? @ghost
Adds test cases for #47949