Skip to content

const fn for option copied, take & replace#86828

Merged
bors merged 1 commit intorust-lang:masterfrom
lambinoo:67441-const-fn-copied-take-replace
Oct 4, 2021
Merged

const fn for option copied, take & replace#86828
bors merged 1 commit intorust-lang:masterfrom
lambinoo:67441-const-fn-copied-take-replace

Conversation

@lambinoo
Copy link
Contributor

@lambinoo lambinoo commented Jul 2, 2021

Tracking issue: #67441

Adding const fn for the copied, take and replace method of Option. Also adding necessary unit test.

It's my first contribution so I am pretty sure I don't know what I'm doing but there's a first for everything!

@rust-highfive
Copy link
Contributor

rust-highfive commented Jul 2, 2021

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

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 Jul 2, 2021
@jonas-schievink
Copy link
Contributor

jonas-schievink commented Jul 2, 2021

I don't think Option::copied works, because const fn cannot have any trait bounds like T: Copy in scope

@lambinoo
Copy link
Contributor Author

lambinoo commented Jul 2, 2021 •
edited
Loading

That's where my knowledge kinda falls short.
But I've assumed that, since the entire impl block where Option::copied is defined is bounded by T: Copy, it should work even with a const fn. Is there something I am missing in this?

@nbdd0121
Copy link
Member

nbdd0121 commented Jul 2, 2021

Trait bound for const fn is gated under #![feature(const_fn_trait_bound)].

@jonas-schievink
Copy link
Contributor

jonas-schievink commented Jul 2, 2021

#86808 also proposes using that feature in libstd, which is still pending decision by the libs team

@lambinoo
Copy link
Contributor Author

lambinoo commented Jul 2, 2021 •
edited
Loading

When it comes to the #![feature(const_fn_trait_bound)] gate, I did not add it myself. Can I assume that it's fine to keep Option::copied in the PR or should it be removed?

fee1-dead
fee1-dead reviewed Jul 3, 2021
View reviewed changes
@lambinoo lambinoo force-pushed the 67441-const-fn-copied-take-replace branch from 7b1bc53 to b08303e Compare July 3, 2021 14:20
ghost
ghost suggested changes Jul 3, 2021
View reviewed changes
@lambinoo lambinoo force-pushed the 67441-const-fn-copied-take-replace branch from a1f5d69 to fac9281 Compare July 3, 2021 16:11
fee1-dead
fee1-dead reviewed Jul 3, 2021
View reviewed changes
@lambinoo lambinoo force-pushed the 67441-const-fn-copied-take-replace branch 3 times, most recently from 2f75b1b to 42447dd Compare July 3, 2021 17:02
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 18, 2021
@lambinoo
Copy link
Contributor Author

lambinoo commented Jul 27, 2021

Hey! Any updates on this?

@bors
Copy link
Collaborator

bors commented Aug 5, 2021

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

@jackh726 jackh726 added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Aug 22, 2021
@lambinoo
const fn for option copied, take & replace + tests
10ddabc 
fix: move test that require mut to another

Adding TODOs for Option::take and Option::copied

TODO to FIXME + moving const stability under normal

Moving const stability attr under normal stab attr

move more rustc stability attributes
@lambinoo lambinoo force-pushed the 67441-const-fn-copied-take-replace branch from 42447dd to 10ddabc Compare August 29, 2021 11:19
@joelpalmer joelpalmer added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 14, 2021
@JohnCSimon
Copy link
Member

JohnCSimon commented Oct 3, 2021

triage:

Hey! Any updates on this?

@lambinoo Josh Triplett seems to have a large review backlog.

@joshtriplett
Copy link
Member

joshtriplett commented Oct 4, 2021

Sorry for the delayed review.

This seems reasonable to me, particularly since all of these are rustc_const_unstable.

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 4, 2021

📌 Commit 10ddabc has been approved by joshtriplett

@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 Oct 4, 2021
Manishearth added a commit to Manishearth/rust that referenced this pull request Oct 4, 2021
@Manishearth
Rollup merge of rust-lang#86828 - lambinoo:67441-const-fn-copied-take…
1c4129d 
…-replace, r=joshtriplett

const fn for option copied, take & replace

Tracking issue: [rust-lang#67441](rust-lang#67441)

Adding const fn for the copied, take and replace method of Option. Also adding necessary unit test.

It's my first contribution so I am pretty sure I don't know what I'm doing but there's a first for everything!
This was referenced Oct 4, 2021
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 4, 2021
@bors
Auto merge of rust-lang#89512 - Manishearth:rollup-meh9x7r, r=Manishe…
44593ae 
…arth

Rollup of 14 pull requests

Successful merges:

 - rust-lang#86434 (Add `Ipv6Addr::is_benchmarking`)
 - rust-lang#86828 (const fn for option copied, take & replace)
 - rust-lang#87679 (BTree: refine some comments)
 - rust-lang#87910 (Mark unsafe methods NonZero*::unchecked_(add|mul) as const.)
 - rust-lang#88286 (Remove unnecessary unsafe block in `process_unix`)
 - rust-lang#88305 (Manual Debug for Unix ExitCode ExitStatus ExitStatusError)
 - rust-lang#88353 (Partially stabilize `array_methods`)
 - rust-lang#88370 (Add missing `# Panics` section to `Vec` method)
 - rust-lang#88481 (Remove some feature gates)
 - rust-lang#89138 (Fix link in Ipv6Addr::to_ipv4 docs)
 - rust-lang#89401 (Add truncate note to Vec::resize)
 - rust-lang#89467 (Fix typos in rustdoc/lints)
 - rust-lang#89472 (Only register `WSACleanup` if `WSAStartup` is actually ever called)
 - rust-lang#89505 (Add regression test for spurious const error with NLL)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 287af04 into rust-lang:master Oct 4, 2021
@rustbot rustbot added this to the 1.57.0 milestone Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fee1-dead fee1-dead fee1-dead left review comments

+1 more reviewer
Reviewers whose approvals may not affect merge requirements

Assignees

@joshtriplett joshtriplett

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.57.0

Development

Successfully merging this pull request may close these issues.