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

Add LinkedList::remove() #68705

Merged
merged 1 commit into from
Feb 20, 2020
Merged

Add LinkedList::remove() #68705

merged 1 commit into from
Feb 20, 2020

Conversation

BijanT
Copy link
Contributor

@BijanT BijanT commented Jan 31, 2020

LinkedList::remove() removes the element at the specified index and returns it.

I added this because I think having a remove function would be useful to have, and similar functions are in other containers, like Vec and HashMap.

I'm not sure if adding a feature like this requires an RFC or not, so I'm sorry if this PR is premature.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @kennytm (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 31, 2020
iter.next_back();
}
iter.tail
};
Copy link
Member

Choose a reason for hiding this comment

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

Could this use the cursors API (currently unstable, but we can use it)? It looks like that should make this a bit cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That seems reasonable. I'll push a commit with that in a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have pushed up the changes

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-7 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2020-01-31T16:30:51.1370101Z ========================== Starting Command Output ===========================
2020-01-31T16:30:51.1372849Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/cd7a2738-33e3-46d3-89f1-5ab3bb20f8c2.sh
2020-01-31T16:30:51.1372891Z 
2020-01-31T16:30:51.1375574Z ##[section]Finishing: Disable git automatic line ending conversion
2020-01-31T16:30:51.1382755Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/68705/merge to s
2020-01-31T16:30:51.1385242Z Task         : Get sources
2020-01-31T16:30:51.1385281Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-01-31T16:30:51.1385308Z Version      : 1.0.0
2020-01-31T16:30:51.1385334Z Author       : Microsoft
---
2020-01-31T16:30:52.0697065Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-01-31T16:30:52.0709122Z ##[command]git config gc.auto 0
2020-01-31T16:30:52.0711402Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-01-31T16:30:52.0713262Z ##[command]git config --get-all http.proxy
2020-01-31T16:30:52.0719697Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/68705/merge:refs/remotes/pull/68705/merge
---
2020-01-31T17:26:24.4777409Z .................................................................................................... 1700/9559
2020-01-31T17:26:29.6050696Z .................................................................................................... 1800/9559
2020-01-31T17:26:42.2454762Z .........................i.......................................................................... 1900/9559
2020-01-31T17:26:49.2257779Z .................................................................................................... 2000/9559
2020-01-31T17:27:03.4883335Z ...............iiiii................................................................................ 2100/9559
2020-01-31T17:27:13.3022046Z .................................................................................................... 2300/9559
2020-01-31T17:27:16.1684431Z .................................................................................................... 2400/9559
2020-01-31T17:27:21.2488781Z .................................................................................................... 2500/9559
2020-01-31T17:27:41.6636348Z .................................................................................................... 2600/9559
---
2020-01-31T17:30:14.5411210Z .................................................................................................... 4800/9559
2020-01-31T17:30:19.6723365Z ..........................................................i...............i......................... 4900/9559
2020-01-31T17:30:27.6306467Z .................................................................................................... 5000/9559
2020-01-31T17:30:35.4234846Z .................................................................................................... 5100/9559
2020-01-31T17:30:40.2303603Z .i.................................................................................................. 5200/9559
2020-01-31T17:30:51.4331983Z ..........................................................................ii.ii........i...i........ 5300/9559
2020-01-31T17:31:00.3546282Z ............i....................................................................................... 5500/9559
2020-01-31T17:31:10.6344169Z .................................................................................................... 5600/9559
2020-01-31T17:31:17.3322514Z .............................................................i...................................... 5700/9559
2020-01-31T17:31:24.8482427Z .................................................................................................... 5800/9559
2020-01-31T17:31:24.8482427Z .................................................................................................... 5800/9559
2020-01-31T17:31:33.0361569Z .................................................................................................... 5900/9559
2020-01-31T17:31:41.9314991Z ....................................................ii...i..ii...........i.......................... 6000/9559
2020-01-31T17:32:03.6241701Z .................................................................................................... 6200/9559
2020-01-31T17:32:11.3837935Z .................................................................................................... 6300/9559
2020-01-31T17:32:11.3837935Z .................................................................................................... 6300/9559
2020-01-31T17:32:19.8600530Z ................................................................................i..ii............... 6400/9559
2020-01-31T17:32:47.5643932Z .................................................................................................... 6600/9559
2020-01-31T17:32:53.4125289Z ........................................................i........................................... 6700/9559
2020-01-31T17:32:55.7428009Z .................................................................................................... 6800/9559
2020-01-31T17:32:58.1311693Z ........................................................i........................................... 6900/9559
---
2020-01-31T17:34:40.3251530Z .................................................................................................... 7600/9559
2020-01-31T17:34:45.3942407Z .................................................................................................... 7700/9559
2020-01-31T17:34:52.0050655Z .................................................................................................... 7800/9559
2020-01-31T17:35:02.2774435Z .................................................................................................... 7900/9559
2020-01-31T17:35:08.1203539Z ............iiiiiii.i............................................................................... 8000/9559
2020-01-31T17:35:22.0637471Z .................................................................................................... 8200/9559
2020-01-31T17:35:32.4125093Z .................................................................................................... 8300/9559
2020-01-31T17:35:45.3897486Z .................................................................................................... 8400/9559
2020-01-31T17:35:52.3357284Z .................................................................................................... 8500/9559
---
2020-01-31T17:38:14.4361927Z  finished in 7.395
2020-01-31T17:38:14.4565576Z Check compiletest suite=codegen mode=codegen (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2020-01-31T17:38:14.6504307Z 
2020-01-31T17:38:14.6505202Z running 169 tests
2020-01-31T17:38:17.6260322Z iiii......i........ii..iiii...i....i...........i............i..i..................i....i............ 100/169
2020-01-31T17:38:19.8329069Z i.i.i...iii..iiiiiiiiii.......................iii............ii......
2020-01-31T17:38:19.8333031Z 
2020-01-31T17:38:19.8333934Z  finished in 5.376
2020-01-31T17:38:19.8528181Z Check compiletest suite=codegen-units mode=codegen-units (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2020-01-31T17:38:20.0175872Z 
---
2020-01-31T17:38:21.9247831Z  finished in 2.072
2020-01-31T17:38:21.9443773Z Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2020-01-31T17:38:22.1059447Z 
2020-01-31T17:38:22.1059656Z running 9 tests
2020-01-31T17:38:22.1060436Z iiiiiiiii
2020-01-31T17:38:22.1060787Z 
2020-01-31T17:38:22.1064706Z  finished in 0.162
2020-01-31T17:38:22.1254370Z Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2020-01-31T17:38:22.3123164Z 
---
2020-01-31T17:38:42.2098867Z  finished in 20.084
2020-01-31T17:38:42.2313993Z Check compiletest suite=debuginfo mode=debuginfo (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2020-01-31T17:38:42.4085998Z 
2020-01-31T17:38:42.4086122Z running 116 tests
2020-01-31T17:38:55.7610264Z iiiii..i.....i..i...i..i.i.i..i...i.ii....i.i....ii..........iiii..........i.....i..i.......ii.i.ii. 100/116
2020-01-31T17:38:57.5720000Z ....iiii.....ii.
2020-01-31T17:38:57.5721072Z 
2020-01-31T17:38:57.5727397Z  finished in 15.341
2020-01-31T17:38:57.5732487Z Uplifting stage1 rustc (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2020-01-31T17:38:57.5733388Z Copying stage2 rustc from stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
---
2020-01-31T17:50:58.2690310Z ---- collections/linked_list.rs - collections::linked_list::LinkedList<T>::remove (line 888) stdout ----
2020-01-31T17:50:58.2690819Z error[E0658]: use of unstable library feature 'linked_list_remove'
2020-01-31T17:50:58.2691181Z   --> collections/linked_list.rs:897:14
2020-01-31T17:50:58.2691330Z    |
2020-01-31T17:50:58.2691666Z 12 | assert_eq!(d.remove(1), Some(2));
2020-01-31T17:50:58.2691950Z    |
2020-01-31T17:50:58.2692096Z    = help: add `#![feature(linked_list_remove)]` to the crate attributes to enable
2020-01-31T17:50:58.2692362Z 
2020-01-31T17:50:58.2692709Z error[E0658]: use of unstable library feature 'linked_list_remove'
2020-01-31T17:50:58.2692709Z error[E0658]: use of unstable library feature 'linked_list_remove'
2020-01-31T17:50:58.2693036Z   --> collections/linked_list.rs:898:14
2020-01-31T17:50:58.2693337Z    |
2020-01-31T17:50:58.2693453Z 13 | assert_eq!(d.remove(0), Some(3));
2020-01-31T17:50:58.2693703Z    |
2020-01-31T17:50:58.2693824Z    = help: add `#![feature(linked_list_remove)]` to the crate attributes to enable
2020-01-31T17:50:58.2693944Z 
2020-01-31T17:50:58.2694281Z error[E0658]: use of unstable library feature 'linked_list_remove'
2020-01-31T17:50:58.2694281Z error[E0658]: use of unstable library feature 'linked_list_remove'
2020-01-31T17:50:58.2694611Z   --> collections/linked_list.rs:899:14
2020-01-31T17:50:58.2694769Z    |
2020-01-31T17:50:58.2694883Z 14 | assert_eq!(d.remove(0), Some(1));
2020-01-31T17:50:58.2695139Z    |
2020-01-31T17:50:58.2695256Z    = help: add `#![feature(linked_list_remove)]` to the crate attributes to enable
2020-01-31T17:50:58.2695356Z 
2020-01-31T17:50:58.2695486Z error: aborting due to 3 previous errors
---
2020-01-31T17:50:58.2858519Z   local time: Fri Jan 31 17:50:58 UTC 2020
2020-01-31T17:50:58.8389429Z   network time: Fri, 31 Jan 2020 17:50:58 GMT
2020-01-31T17:50:58.8390495Z == end clock drift check ==
2020-01-31T17:50:59.2127877Z 
2020-01-31T17:50:59.2253506Z ##[error]Bash exited with code '1'.
2020-01-31T17:50:59.2266528Z ##[section]Finishing: Run build
2020-01-31T17:50:59.2299198Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/68705/merge to s
2020-01-31T17:50:59.2301442Z Task         : Get sources
2020-01-31T17:50:59.2301487Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-01-31T17:50:59.2301570Z Version      : 1.0.0
2020-01-31T17:50:59.2301609Z Author       : Microsoft
2020-01-31T17:50:59.2301609Z Author       : Microsoft
2020-01-31T17:50:59.2301652Z Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
2020-01-31T17:50:59.2301736Z ==============================================================================
2020-01-31T17:50:59.6447737Z Cleaning any cached credential from repository: rust-lang/rust (GitHub)
2020-01-31T17:50:59.6491623Z ##[section]Finishing: Checkout rust-lang/rust@refs/pull/68705/merge to s
2020-01-31T17:50:59.6603743Z Cleaning up task key
2020-01-31T17:50:59.6604453Z Start cleaning up orphan processes.
2020-01-31T17:50:59.6705515Z Terminate orphan process: pid (3877) (python)
2020-01-31T17:50:59.6938140Z ##[section]Finishing: Finalize Job

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

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

Looking much better, left a few more comments though.

src/liballoc/collections/linked_list.rs Show resolved Hide resolved
src/liballoc/collections/linked_list.rs Show resolved Hide resolved
cursor.remove_current()
} else {
let mut cursor = self.cursor_back_mut();
for _ in 0..len - at - 1 {
Copy link
Member

Choose a reason for hiding this comment

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

Can we pull out len - at - 1 to e.g. "offset_from_end" or something like that?

@BijanT
Copy link
Contributor Author

BijanT commented Feb 5, 2020

I've made the requested changes

@JohnTitor
Copy link
Member

r? @Mark-Simulacrum

Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

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

r=me with a tracking issue filed and added, as well as commits squashed; thanks! Sorry for the delay in review.

@kennytm kennytm 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 Feb 16, 2020
@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 17, 2020
Copy link
Contributor

@mlodato517 mlodato517 left a comment

Choose a reason for hiding this comment

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

Threw out some potential re-wordings that you should feel free to ignore :-) And asked some questions so I can learn more about rust and writing code for others :-)

src/liballoc/collections/linked_list.rs Show resolved Hide resolved
src/liballoc/collections/linked_list.rs Show resolved Hide resolved
src/liballoc/collections/linked_list.rs Outdated Show resolved Hide resolved
src/liballoc/collections/linked_list.rs Outdated Show resolved Hide resolved
src/liballoc/collections/linked_list.rs Outdated Show resolved Hide resolved
@Mark-Simulacrum
Copy link
Member

r=me with the comments resolved or not, just let me know if you want to fix any of them in this PR (if not I personally think this is fine to land without fixing them too)

LinkedList::remove() removes the element at the specified index and returns it.

Signed-off-by: Bijan Tabatabai <bijan311@yahoo.com>
@BijanT
Copy link
Contributor Author

BijanT commented Feb 19, 2020

I made some of the changes that mlodato517 suggested. This PR should be good to go now

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

Thanks!

@bors
Copy link
Contributor

bors commented Feb 19, 2020

📌 Commit c797ce7 has been approved by Mark-Simulacrum

@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 Feb 19, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 20, 2020
Add LinkedList::remove()

LinkedList::remove() removes the element at the specified index and returns it.

I added this because I think having a remove function would be useful to have, and similar functions are in other containers, like Vec and HashMap.

I'm not sure if adding a feature like this requires an RFC or not, so I'm sorry if this PR is premature.
bors added a commit that referenced this pull request Feb 20, 2020
Rollup of 5 pull requests

Successful merges:

 - #68705 (Add LinkedList::remove())
 - #68945 (Stabilize Once::is_completed)
 - #68978 (Make integer exponentiation methods unstably const)
 - #69266 (Fix race condition when allocating source files in SourceMap)
 - #69287 (Clean up E0317 explanation)

Failed merges:

r? @ghost
@bors bors merged commit f7ce5ff into rust-lang:master Feb 20, 2020
@BijanT BijanT deleted the ll_remove branch April 23, 2020 15:36
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants