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.

Sign up for GitHub

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

Jump to bottom

turn ManuallyDrop::new into a constant function #50148

Merged
merged 2 commits into from
May 9, 2018
Merged

turn ManuallyDrop::new into a constant function #50148

merged 2 commits into from
May 9, 2018

Conversation

japaric
Copy link
Member

@japaric japaric commented Apr 21, 2018

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 21, 2018
@sfackler sfackler added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Apr 21, 2018
@sfackler
Copy link
Member

@rfcbot fcp merge

This seems reasonable to me but let's make sure everyone's on board since it's insta-stable.

@rfcbot
Copy link

rfcbot commented Apr 21, 2018 •
edited by Kimundi
Loading

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

No concerns currently listed.

Once a majority of reviewers approve (and none object), 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 rfcbot added the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Apr 21, 2018
@sfackler sfackler added the relnotes Marks issues that should be documented in the release notes of the next release. label Apr 21, 2018
@SimonSapin
Copy link
Contributor

This sounds fine, but don’t you need some other APIs to be const as well for this to be useful? What’s an usage example?

@japaric
Copy link
Member Author

japaric commented Apr 23, 2018

@SimonSapin

don’t you need some other APIs to be const as well for this to be useful?

Yes, mem::uninitialized (cf. #50150). But I just heard on that other PR that the plan is to deprecate mem::uninitialized in favor of MaybeUninit. Personally, I'd be OK with using MaybeUninit instead of ManuallyDrop + mem::uninitialized if the RFC gets accepted and/or if there concerns about insta-stabilizing ManuallyDrop::new. (In any case, I'm stuck with nightly until const_fn is stabilized).

What’s an usage example?

Fixed capacity dynamic data structures (Vec, String, HashMap, etc) that you can put in static variables. See heapless.

@SimonSapin
Copy link
Contributor

SimonSapin commented Apr 23, 2018 •
edited
Loading

What I had in mind is: it seems that ManuallyDrop::new is only useful if you can do anything with that value of type ManuallyDrop afterwards. The main APIs for that are Deref and DerefMut, but they’re not const. I’ve just now realized that you only want const to initialize a static item, and then use it at runtime.

This is a long way of saying: sounds good, never mind my previous comment :)

@aturon
Copy link
Member

aturon commented Apr 25, 2018

@bors: rollup

@rfcbot
Copy link

rfcbot commented Apr 25, 2018

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

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Apr 25, 2018
@kennytm
Copy link
Member

kennytm commented Apr 25, 2018

You could make this not insta-stable via rustc_const_unstable. See TypeId::of.

@oli-obk
Copy link
Contributor

oli-obk commented May 3, 2018

Ping @japaric

@rfcbot
Copy link

rfcbot commented May 5, 2018

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

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels May 5, 2018
@oli-obk
Copy link
Contributor

oli-obk commented May 7, 2018

@bors r+

@bors
Copy link
Contributor

bors commented May 7, 2018

‼️ Invalid head SHA found, retrying: 0000000000000000000000000000000000000000

@bors
Copy link
Contributor

bors commented May 7, 2018

📌 Commit b61a4c2 has been approved by oli-obk

@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 May 7, 2018
@kennytm kennytm mentioned this pull request May 8, 2018
Merged
kennytm added a commit to kennytm/rust that referenced this pull request May 9, 2018
@kennytm
Rollup merge of rust-lang#50148 - japaric:const-manuallydrop, r=oli-obk
dea03f1 
turn `ManuallyDrop::new` into a constant function
bors added a commit that referenced this pull request May 9, 2018
@bors
Auto merge of #50546 - kennytm:rollup, r=kennytm
ac287ed 
Rollup of 11 pull requests

Successful merges:

 - #49988 (Mention Result<!, E> in never docs.)
 - #50148 (turn `ManuallyDrop::new` into a constant function)
 - #50456 (Update the Cargo submodule)
 - #50460 (Make `String::new()` const)
 - #50464 (Remove some transmutes)
 - #50505 (Added regression function match value test)
 - #50511 (Add some explanations for #[must_use])
 - #50525 (Optimize string handling in lit_token().)
 - #50527 (Cleanup a `use` in a raw_vec test)
 - #50539 (Add more logarithm constants)
 - #49523 (Update RELEASES.md for 1.26.0)

Failed merges:
@bors bors merged commit b61a4c2 into rust-lang:master May 9, 2018
@XAMPPRocky XAMPPRocky removed the relnotes Marks issues that should be documented in the release notes of the next release. label Jun 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@sfackler sfackler

Labels
finished-final-comment-period The final comment period is finished for this PR / Issue. 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
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@japaric @sfackler @rfcbot @SimonSapin @aturon @kennytm @oli-obk @bors @XAMPPRocky @rust-highfive