Add #[track_caller] to panicking Vec functions #79323
Closed
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
When allocation fails, it's very helpful to know where in user code.
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
Seems reasonable to me. r=me if CI passes. |
|
@bors rollup=never just in case this has perf effects, Vec is hot. |
|
☔ The latest upstream changes (presumably #79186) made this pull request unmergeable. Please resolve the merge conflicts. Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels: |
crlf0710
added
S-waiting-on-author
|
@nvzqz Triage: there's merge conflicts now. |
JohnCSimon
added
S-waiting-on-author
|
Pinging again from triage: |
|
Triage - I'm closing this as inactive |
JohnCSimon
added
the
S-inactive
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 16, 2024
Add `#[track_caller]` to allocating methods of `Vec` & `VecDeque` Part 4 in a lengthy saga. r? `@joshtriplett` because they were the reviewer the last 3 times. `@bors` rollup=never "[just in case this has perf effects, Vec is hot](rust-lang#79323 (comment))" This was first attempted in rust-lang#79323 by `@nvzqz.` It got approval from `@joshtriplett,` but rotted with merge conflicts and got closed. Then it got picked up by `@Dylan-DPC-zz` in rust-lang#83359. A benchmark was run[^perf], the results (after a bit of thinking[^thinking]) were deemed ok[^ok], but there was a typo[^typo] and the PR was made from a wrong remote in the first place[^remote], so rust-lang#83909 was opened instead. By the time rust-lang#83909 rolled around, the methods in question had received some optimizations[^optimizations], so another perf run was conducted[^perf2]. The results were ok[^ok2]. There was a suggestion to add regression tests for panic behavior [^tests], but before it could be addressed, the PR fell victim to merge conflicts[^conflicts] and died again[^rip]. 3 years have passed, and (from what I can tell) this has not been tried again, so here I am now, reviving this old effort. Given how much time has passed and the fact that I've also touched `VecDeque` this time, it probably makes sense to `@bors` try `@rust-timer` [^perf]: rust-lang#83359 (comment) [^thinking]: rust-lang#83359 (comment) [^ok]: rust-lang#83359 (comment) [^typo]: rust-lang#83359 (comment) [^remote]: rust-lang#83359 (comment) [^optimizations]: rust-lang#83909 (comment) [^perf2]: rust-lang#83909 (comment) [^ok2]: rust-lang#83909 (comment) [^tests]: rust-lang#83909 (comment) [^conflicts]: rust-lang#83909 (comment) [^rip]: rust-lang#83909 (comment)
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 21, 2024
Add `#[track_caller]` to allocating methods of `Vec` & `VecDeque` Part 4 in a lengthy saga. r? `@joshtriplett` because they were the reviewer the last 3 times. `@bors` rollup=never "[just in case this has perf effects, Vec is hot](rust-lang#79323 (comment))" This was first attempted in rust-lang#79323 by `@nvzqz.` It got approval from `@joshtriplett,` but rotted with merge conflicts and got closed. Then it got picked up by `@Dylan-DPC-zz` in rust-lang#83359. A benchmark was run[^perf], the results (after a bit of thinking[^thinking]) were deemed ok[^ok], but there was a typo[^typo] and the PR was made from a wrong remote in the first place[^remote], so rust-lang#83909 was opened instead. By the time rust-lang#83909 rolled around, the methods in question had received some optimizations[^optimizations], so another perf run was conducted[^perf2]. The results were ok[^ok2]. There was a suggestion to add regression tests for panic behavior [^tests], but before it could be addressed, the PR fell victim to merge conflicts[^conflicts] and died again[^rip]. 3 years have passed, and (from what I can tell) this has not been tried again, so here I am now, reviving this old effort. Given how much time has passed and the fact that I've also touched `VecDeque` this time, it probably makes sense to `@bors` try `@rust-timer` [^perf]: rust-lang#83359 (comment) [^thinking]: rust-lang#83359 (comment) [^ok]: rust-lang#83359 (comment) [^typo]: rust-lang#83359 (comment) [^remote]: rust-lang#83359 (comment) [^optimizations]: rust-lang#83909 (comment) [^perf2]: rust-lang#83909 (comment) [^ok2]: rust-lang#83909 (comment) [^tests]: rust-lang#83909 (comment) [^conflicts]: rust-lang#83909 (comment) [^rip]: rust-lang#83909 (comment)
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 14, 2024
Add `#[track_caller]` to allocating methods of `Vec` & `VecDeque` Part 4 in a lengthy saga. r? `@joshtriplett` because they were the reviewer the last 3 times. `@bors` rollup=never "[just in case this has perf effects, Vec is hot](rust-lang#79323 (comment))" This was first attempted in rust-lang#79323 by `@nvzqz.` It got approval from `@joshtriplett,` but rotted with merge conflicts and got closed. Then it got picked up by `@Dylan-DPC-zz` in rust-lang#83359. A benchmark was run[^perf], the results (after a bit of thinking[^thinking]) were deemed ok[^ok], but there was a typo[^typo] and the PR was made from a wrong remote in the first place[^remote], so rust-lang#83909 was opened instead. By the time rust-lang#83909 rolled around, the methods in question had received some optimizations[^optimizations], so another perf run was conducted[^perf2]. The results were ok[^ok2]. There was a suggestion to add regression tests for panic behavior [^tests], but before it could be addressed, the PR fell victim to merge conflicts[^conflicts] and died again[^rip]. 3 years have passed, and (from what I can tell) this has not been tried again, so here I am now, reviving this old effort. Given how much time has passed and the fact that I've also touched `VecDeque` this time, it probably makes sense to `@bors` try `@rust-timer` [^perf]: rust-lang#83359 (comment) [^thinking]: rust-lang#83359 (comment) [^ok]: rust-lang#83359 (comment) [^typo]: rust-lang#83359 (comment) [^remote]: rust-lang#83359 (comment) [^optimizations]: rust-lang#83909 (comment) [^perf2]: rust-lang#83909 (comment) [^ok2]: rust-lang#83909 (comment) [^tests]: rust-lang#83909 (comment) [^conflicts]: rust-lang#83909 (comment) [^rip]: rust-lang#83909 (comment)
RalfJung
pushed a commit
to RalfJung/miri
that referenced
this pull request
Oct 14, 2024
Add `#[track_caller]` to allocating methods of `Vec` & `VecDeque` Part 4 in a lengthy saga. r? `@joshtriplett` because they were the reviewer the last 3 times. `@bors` rollup=never "[just in case this has perf effects, Vec is hot](rust-lang/rust#79323 (comment))" This was first attempted in #79323 by `@nvzqz.` It got approval from `@joshtriplett,` but rotted with merge conflicts and got closed. Then it got picked up by `@Dylan-DPC-zz` in #83359. A benchmark was run[^perf], the results (after a bit of thinking[^thinking]) were deemed ok[^ok], but there was a typo[^typo] and the PR was made from a wrong remote in the first place[^remote], so #83909 was opened instead. By the time #83909 rolled around, the methods in question had received some optimizations[^optimizations], so another perf run was conducted[^perf2]. The results were ok[^ok2]. There was a suggestion to add regression tests for panic behavior [^tests], but before it could be addressed, the PR fell victim to merge conflicts[^conflicts] and died again[^rip]. 3 years have passed, and (from what I can tell) this has not been tried again, so here I am now, reviving this old effort. Given how much time has passed and the fact that I've also touched `VecDeque` this time, it probably makes sense to `@bors` try `@rust-timer` [^perf]: rust-lang/rust#83359 (comment) [^thinking]: rust-lang/rust#83359 (comment) [^ok]: rust-lang/rust#83359 (comment) [^typo]: rust-lang/rust#83359 (comment) [^remote]: rust-lang/rust#83359 (comment) [^optimizations]: rust-lang/rust#83909 (comment) [^perf2]: rust-lang/rust#83909 (comment) [^ok2]: rust-lang/rust#83909 (comment) [^tests]: rust-lang/rust#83909 (comment) [^conflicts]: rust-lang/rust#83909 (comment) [^rip]: rust-lang/rust#83909 (comment)
lnicola
pushed a commit
to lnicola/rust-analyzer
that referenced
this pull request
Oct 17, 2024
Add `#[track_caller]` to allocating methods of `Vec` & `VecDeque` Part 4 in a lengthy saga. r? `@joshtriplett` because they were the reviewer the last 3 times. `@bors` rollup=never "[just in case this has perf effects, Vec is hot](rust-lang/rust#79323 (comment))" This was first attempted in #79323 by `@nvzqz.` It got approval from `@joshtriplett,` but rotted with merge conflicts and got closed. Then it got picked up by `@Dylan-DPC-zz` in #83359. A benchmark was run[^perf], the results (after a bit of thinking[^thinking]) were deemed ok[^ok], but there was a typo[^typo] and the PR was made from a wrong remote in the first place[^remote], so #83909 was opened instead. By the time #83909 rolled around, the methods in question had received some optimizations[^optimizations], so another perf run was conducted[^perf2]. The results were ok[^ok2]. There was a suggestion to add regression tests for panic behavior [^tests], but before it could be addressed, the PR fell victim to merge conflicts[^conflicts] and died again[^rip]. 3 years have passed, and (from what I can tell) this has not been tried again, so here I am now, reviving this old effort. Given how much time has passed and the fact that I've also touched `VecDeque` this time, it probably makes sense to `@bors` try `@rust-timer` [^perf]: rust-lang/rust#83359 (comment) [^thinking]: rust-lang/rust#83359 (comment) [^ok]: rust-lang/rust#83359 (comment) [^typo]: rust-lang/rust#83359 (comment) [^remote]: rust-lang/rust#83359 (comment) [^optimizations]: rust-lang/rust#83909 (comment) [^perf2]: rust-lang/rust#83909 (comment) [^ok2]: rust-lang/rust#83909 (comment) [^tests]: rust-lang/rust#83909 (comment) [^conflicts]: rust-lang/rust#83909 (comment) [^rip]: rust-lang/rust#83909 (comment)
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.
When allocation fails, it's very helpful to know where in user code.