Skip to content
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

Make IdFunctor::try_map_id panic-safe #91426

Merged
merged 1 commit into from
Dec 11, 2021

Conversation

eggyal
Copy link
Contributor

@eggyal eggyal commented Dec 1, 2021

Addresses FIXME comment created in #78313

r? @lcnr

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 1, 2021
@rust-log-analyzer

This comment has been minimized.

@eggyal eggyal marked this pull request as draft December 1, 2021 18:19
@rust-log-analyzer

This comment has been minimized.

@jackh726
Copy link
Member

jackh726 commented Dec 1, 2021

squints "free(): double free detected in tcache 2"

I think there's a bug here 😂

@eggyal
Copy link
Contributor Author

eggyal commented Dec 1, 2021

I think there's a bug here 😂

Aye, took me a while to spot it, but the assignment **slot = f(..)? was of course dropping the already-moved-out T-element.

@eggyal eggyal marked this pull request as ready for review December 1, 2021 22:55
@bors
Copy link
Contributor

bors commented Dec 2, 2021

☔ The latest upstream changes (presumably #91318) made this pull request unmergeable. Please resolve the merge conflicts.

@lcnr
Copy link
Contributor

lcnr commented Dec 5, 2021

this does look right to me, but stuff like that's scary, so if someone else from @rust-lang/compiler-contributors wants to look at this, please do ✨

if not, i am going to approve this in a few days

@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 9, 2021
@lcnr
Copy link
Contributor

lcnr commented Dec 9, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Dec 9, 2021

📌 Commit acd39ff has been approved by lcnr

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 9, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 9, 2021
Make IdFunctor::try_map_id panic-safe

Addresses FIXME comment created in rust-lang#78313

r? `@lcnr`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 10, 2021
Make IdFunctor::try_map_id panic-safe

Addresses FIXME comment created in rust-lang#78313

r? ``@lcnr``
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 11, 2021
…askrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#91127 (Add `<*{const|mut} T>::{to|from}_bits`)
 - rust-lang#91310 (Add --out-dir flag for rustdoc)
 - rust-lang#91373 (Add needs-unwind to tests that depend on panicking)
 - rust-lang#91426 (Make IdFunctor::try_map_id panic-safe)
 - rust-lang#91515 (Add rsplit_array variants to slices and arrays)
 - rust-lang#91553 (socket ancillary data implementation for dragonflybsd.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7e18b79 into rust-lang:master Dec 11, 2021
@rustbot rustbot added this to the 1.59.0 milestone Dec 11, 2021
@eggyal eggyal deleted the idfunctor-panic-safety branch December 12, 2021 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants