Skip to content

BTreeMap: Support custom allocators#77438

Closed
exrook wants to merge 4 commits into
rust-lang:masterfrom
exrook:btreemap-alloc
Closed

BTreeMap: Support custom allocators#77438
exrook wants to merge 4 commits into
rust-lang:masterfrom
exrook:btreemap-alloc

Conversation

@exrook

@exrook exrook commented Oct 2, 2020

Copy link
Copy Markdown
Contributor

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 2, 2020
@bors

bors commented Oct 2, 2020

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #77436) 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:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@exrook exrook force-pushed the btreemap-alloc branch 2 times, most recently from 41c0200 to c7237fa Compare October 2, 2020 04:13
@exrook

exrook commented Oct 2, 2020

Copy link
Copy Markdown
Contributor Author

@rustbot modify labels: +A-allocators +T-libs

@rustbot rustbot added A-allocators Area: Custom and system allocators T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Oct 2, 2020
@bors

bors commented Oct 3, 2020

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #77470) 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:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@TimDiekmann

This comment has been minimized.

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 3, 2020
Comment thread library/alloc/src/collections/btree/map.rs Outdated
Comment thread library/alloc/src/collections/btree/map.rs Outdated
@TimDiekmann

Copy link
Copy Markdown
Member

cc @Amanieu

@cramertj

cramertj commented Oct 5, 2020

Copy link
Copy Markdown
Member

r? @Amanieu

@bors

bors commented Oct 15, 2020

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #77981) 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:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 16, 2020
BTreeMap: refactor Entry out of map.rs into its own file

btree/map.rs is approaching the 3000 line mark, splitting out the entry
code buys about 500 lines of headroom.

I've created this PR because the changes I've made in rust-lang#77438 will push `map.rs` over the 3000 line limit and cause tidy to complain.

I picked `Entry` to factor out because it feels less tightly coupled to the rest of `BTreeMap` than the various iterator implementations.

Related: rust-lang#60302
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 16, 2020
BTreeMap: refactor Entry out of map.rs into its own file

btree/map.rs is approaching the 3000 line mark, splitting out the entry
code buys about 500 lines of headroom.

I've created this PR because the changes I've made in rust-lang#77438 will push `map.rs` over the 3000 line limit and cause tidy to complain.

I picked `Entry` to factor out because it feels less tightly coupled to the rest of `BTreeMap` than the various iterator implementations.

Related: rust-lang#60302
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 17, 2020
BTreeMap: refactor Entry out of map.rs into its own file

btree/map.rs is approaching the 3000 line mark, splitting out the entry
code buys about 500 lines of headroom.

I've created this PR because the changes I've made in rust-lang#77438 will push `map.rs` over the 3000 line limit and cause tidy to complain.

I picked `Entry` to factor out because it feels less tightly coupled to the rest of `BTreeMap` than the various iterator implementations.

Related: rust-lang#60302
@bors

bors commented Oct 17, 2020

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #78060) 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:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@bors

bors commented Oct 21, 2020

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #77244) 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:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@rust-log-analyzer

This comment has been minimized.

@exrook exrook marked this pull request as ready for review March 23, 2022 12:00
@bors

bors commented Apr 6, 2022

Copy link
Copy Markdown
Collaborator

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

@JohnCSimon

Copy link
Copy Markdown
Member

Triage:
@Amanieu is this ready for review?

@Amanieu Amanieu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about the late review, this looks good modulo the leftover test code. Can you rebase to resolve the conflicts?

Comment thread src/bootstrap/builder.rs
rustflags.arg(&format!("-Cllvm-args=-import-instr-limit={}", limit));
}
}
cargo.env(profile_var("LTO"), "off");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leftover from testing?

@Dylan-DPC Dylan-DPC added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 24, 2022
@TennyZhuang

Copy link
Copy Markdown
Contributor

What's the status of the PR? I'd like to help with something.

@Amanieu

Amanieu commented Jun 9, 2022

Copy link
Copy Markdown
Member

@TennyZhuang Since the author is not responding, you can take over by forking their code and opening a new PR.

@TennyZhuang

Copy link
Copy Markdown
Contributor

@exrook Sorry but I really need the feature, I'll take over the PR at the weekend. 🥰

@TennyZhuang

Copy link
Copy Markdown
Contributor

@Amanieu Hi, I've resolved the conflicts and submitted a new PR #98015, PTAL Thanks!

@Dylan-DPC

Copy link
Copy Markdown
Member

Closing this in favour of #98015

@Dylan-DPC Dylan-DPC closed this Jun 12, 2022
@exrook

exrook commented Jun 12, 2022

Copy link
Copy Markdown
Contributor Author

@TennyZhuang I'm glad to hear you're also looking to use this feature! My apologies for not responding sooner, I've been busy with other things, and when I last attempted a rebase on master back in May I was still hitting box-related ICEs so I've been working on a branch based on top of #95576, though it seems now those ICEs have been fixed in master :).

@Dylan-DPC Hi, could this be re-opened? I've put a lot of effort into keeping these changes in sync with master over the past year+ and a half and I'd like to see it through to being merged. :)

@Amanieu Thanks for the review! I've rebased on master and removed the leftover test code, so this should be good to go if you have no other comments

@TennyZhuang

Copy link
Copy Markdown
Contributor

@exrook I’m glad to hear that, and really hope the PR can be merged ASAP.
@Dylan-DPC @Amanieu Can you help to reopen the PR and review it again? Sorry for the extra review work.

@Amanieu

Amanieu commented Jun 13, 2022

Copy link
Copy Markdown
Member

GIthub doesn't let me reopen the PR after a force-push/rebase. You'll need to open a new one.

@TennyZhuang

TennyZhuang commented Jun 13, 2022

Copy link
Copy Markdown
Contributor

@exrook I’ve found that it’s possible to reopen the PR, but you have to revert the commits back to the one before closed, and commit them again, sorry.

See https://gist.github.com/robertpainsi/2c42c15f1ce6dab03a0675348edd4e2c

Feel free to reopen the PR or just create a new one.

@Dylan-DPC

Copy link
Copy Markdown
Member

@exrook unfortunately you can't reöpen this. You can submit a new pull request and link this one in the description and we can take it from there

@exrook

exrook commented Jun 14, 2022

Copy link
Copy Markdown
Contributor Author

Thanks everyone! I've opened a new PR at #98103

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 15, 2022
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 15, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-allocators Area: Custom and system allocators S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.