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 a system for explicitly marking const fns as promotable #51500

Closed
wants to merge 7 commits into from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jun 11, 2018

@rust-highfive
Copy link
Collaborator

warning Warning warning

  • These commits modify submodules.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 11, 2018
@oli-obk oli-obk force-pushed the 2const_or_¬2const branch from 4333d6e to bb88df8 Compare June 11, 2018 16:24
@gnzlbg
Copy link
Contributor

gnzlbg commented Jun 11, 2018

As mentioned on the stdsimd PR (rust-lang/stdarch#471): the feature has no tracking issue, neither the PR nor the commits explain what it does, nor there are any docs... yet std library APIs need to be marked with the attribute.

I don't know where the right place to document this is, but I think it should be explained somewhere (and maybe the libs team should be pinged here as well), since std maintainers need to know what this is and how to use it.

@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 11, 2018

The idea is that you do not need to mark anything as promotable. We just needed an opt-in for preventing the automatic promotion of any const fn call due to #50814 . Without that opt in, further stabilization of const fns is not something we want to do.

@rust-highfive

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@bors

This comment has been minimized.

@oli-obk oli-obk force-pushed the 2const_or_¬2const branch from b5bd6a1 to 116df85 Compare June 12, 2018 13:43
@rust-highfive

This comment has been minimized.

@@ -929,10 +935,18 @@ This does not pose a problem by itself because they can't be accessed directly."

_ => {}
}
// we can always mark intrinsics as promotable as they are inherently
// unstable
promotable = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

let's extend the "promotable" whitelist to intrinsics too

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

I approve of the general approach — but I agree that we should document. I was gonna say the same thing. @oli-obk and I talked and agreed we'd put it in the unstable reference.

@nikomatsakis
Copy link
Contributor

cc @eddyb — do you want to take a look at this PR? (Not strictly necessary, I don't think) It adds a separate attribute to mark const fns as promotable.


@gnzlbg as I wrote above, I agree we need docs — but I think the short version is that we would only very rarely (if ever) want to add this attribute, and only after consulting with @oli-obk and/or myself. Basically we want to be conservative about what we will automatically promote to static lifetime (that is, you write let x = &foo and we allow that to escape the fn) until we've fully worked out the rules.

@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 13, 2018

cc @eddyb

@rust-highfive

This comment has been minimized.

@oli-obk oli-obk force-pushed the 2const_or_¬2const branch from b91bcc0 to a810d95 Compare June 14, 2018 05:47
@eddyb
Copy link
Member

eddyb commented Jun 14, 2018

I'm not sure we should mark any const fn to be promotable, if it does anything to its arguments other than moving them into the return value (usually some sort of ADT).
There is a risk that any operation may fail when applied to an unknown value.

@pietroalbini
Copy link
Member

Ping from triage @nikomatsakis! This PR needs your review.

@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 18, 2018

closing in favour of #51570

@oli-obk oli-obk closed this Jun 18, 2018
@oli-obk oli-obk deleted the 2const_or_¬2const branch June 15, 2020 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants