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

Constify LinkedList new function #63684

Merged
merged 1 commit into from Aug 31, 2019

Conversation

@GrayJack
Copy link
Contributor

commented Aug 18, 2019

Change the LinkedList::new() function to become a const fn, allowing the use in constant context.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Aug 18, 2019

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cramertj (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.

@Centril

This comment has been minimized.

Copy link
Member

commented Aug 18, 2019

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

@GrayJack FWIW could you perhaps elaborate on this PR a little bit? There's nothing in the PR description, nothing else in the commit except the title of the PR, and the change simply deletes one word. While self-explanatory enough it's generally good hygiene to have at least a few words motivating the change instead of just leaving it up to readers to figure out what's going on.

@rfcbot fcp merge

@rfcbot

This comment has been minimized.

Copy link

commented Aug 19, 2019

Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot

This comment has been minimized.

Copy link

commented Aug 19, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@daboross

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

Just wondering: is the lack of unsafety here why this function can be insta-stable const while Vec::new remains marked const_unstable?

Vec::new was similarly marked const, but it was not marked stable:

rust/src/liballoc/vec.rs

Lines 316 to 317 in c422372

#[rustc_const_unstable(feature = "const_vec_new")]
pub const fn new() -> Vec<T> {

@GrayJack

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2019

@Centril said that Vec is under is blocked for other reason, but I have no more background to know what blocks it.

If in the end is decided that it should be behind a feature gate, I'll gladly do it

@rfcbot

This comment has been minimized.

Copy link

commented Aug 29, 2019

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@Centril

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

r? @Centril @bors r+ rollup

@bors

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2019

📌 Commit e0f7305 has been approved by Centril

Centril added a commit to Centril/rust that referenced this pull request Aug 30, 2019
Rollup merge of rust-lang#63684 - GrayJack:const_list_new, r=Centril
Constify LinkedList new function

Change the `LinkedList::new()` function to become a const fn, allowing the use in constant context.
bors added a commit that referenced this pull request Aug 30, 2019
Auto merge of #64012 - Centril:rollup-cukawo8, r=Centril
Rollup of 4 pull requests

Successful merges:

 - #61626 (Get rid of special const intrinsic query in favour of `const_eval`)
 - #63600 (Merge oli-obk mail addresses)
 - #63684 (Constify LinkedList new function)
 - #63982 (When accessing private field of union, do not misidentify it as a struct)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Aug 30, 2019
Rollup merge of rust-lang#63684 - GrayJack:const_list_new, r=Centril
Constify LinkedList new function

Change the `LinkedList::new()` function to become a const fn, allowing the use in constant context.
bors added a commit that referenced this pull request Aug 30, 2019
Auto merge of #64026 - Centril:rollup-le667lp, r=Centril
Rollup of 7 pull requests

Successful merges:

 - #62957 (Match the loop examples)
 - #63600 (Merge oli-obk mail addresses)
 - #63684 (Constify LinkedList new function)
 - #63847 ([rustdoc] Fix system theme detection)
 - #63999 (Add missing links on AsRef trait)
 - #64014 ( miri: detect too large dynamically sized objects )
 - #64015 (some const-eval test tweaks)

Failed merges:

r? @ghost

@bors bors merged commit e0f7305 into rust-lang:master Aug 31, 2019

4 checks passed

pr Build #20190818.12 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-6.0) Linux x86_64-gnu-llvm-6.0 succeeded
Details
pr (LinuxTools) LinuxTools succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.