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

const Path initialisation #92930

Closed
wants to merge 5 commits into from

Conversation

conradludgate
Copy link
Contributor

@conradludgate conradludgate commented Jan 15, 2022

Path::new cannot be made const currently, due to its usage of AsRef<OsStr>.

Since Rust does not yet have any concept of const traits, this PR adds a couple new methods in order to construct const Paths.

The expected usage is

const PATH: &Path = Path::from_os_str(OsStr::from_str("/my/path"));

Alternatives

In some future, we may get const traits, such that we could do

impl const From<&str> for &Path {
    // ...
}

const PATH: &Path = Path::from("/my/path");

@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 @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 Jan 15, 2022
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@joshtriplett joshtriplett added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jan 15, 2022
@joshtriplett
Copy link
Member

I think it may sometimes make sense to have methods like this regardless, both for self-documenting code and to usefully constrain inference.

To evaluate whether these methods carry enough weight to add, it would help to know roughly when enough of const traits will become available to make these methods unnecessary for that purpose.

cc @oli-obk @rust-lang/wg-const-eval; do you have a rough idea of the timeline upon which we can support const traits well enough to not need these methods? That would help determine whether we want to add workarounds like this.

@fee1-dead
Copy link
Member

fee1-dead commented Jan 15, 2022

Perhaps a sane way is to stabilize a subset of const_trait_impl that only allows calling functions from concrete const implementations of a trait. We could save defining a impl const, calling/defining a function that uses const trait bounds for stabilization later and enable uses like this from the standard library.

@bors
Copy link
Contributor

bors commented Feb 13, 2022

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

@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 Mar 20, 2022
@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 Apr 24, 2022
@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 May 22, 2022
@Dylan-DPC
Copy link
Member

@conradludgate if you can resolve the conflicts we can push this forward

@Dylan-DPC Dylan-DPC 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 May 23, 2022
@JohnCSimon JohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 19, 2022
@Dylan-DPC Dylan-DPC 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 Jul 11, 2022
@Mark-Simulacrum Mark-Simulacrum added the A-const-eval Area: Constant evaluation (MIR interpretation) label Aug 6, 2022
@joshtriplett
Copy link
Member

joshtriplett commented Oct 3, 2022

This looks reasonable to me.

It seems like the common case will be making a const path from a str, rather than an OsStr. I think it would make sense to add a method for doing that directly. Such a method probably shouldn't use the name from_str since that would conflict with FromStr::from_str, but I think a method should exist for that to simplify the common case.

cc @rust-lang/libs-api: name suggestions?

Also, could you please file a tracking issue and use it in the stability attributes?

#[inline]
#[unstable(feature = "const_path", reason = "TBD", issue = "none")]
pub const fn from_os_str(s: &OsStr) -> &Path {
unsafe { &*(s as *const OsStr as *const Path) }
Copy link
Member

Choose a reason for hiding this comment

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

osstr.as_ref() -> Path already exists. Imo it's better to wait until that can be made const before introducing additional API surface for the sole purpose of supporting constification.

@pitaj
Copy link
Contributor

pitaj commented Apr 30, 2023

Ping from triage. @conradludgate you still need to reference the tracking issue in your stability attributes.

@joshtriplett It appears there are mixed reactions, are you still interested in these additions?

@pitaj pitaj 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 Apr 30, 2023
@JohnCSimon
Copy link
Member

@conradludgate

Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you are going to continue please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive

@JohnCSimon JohnCSimon closed this Oct 1, 2023
@rustbot rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Oct 1, 2023
@ferdonline
Copy link

I find this really useful. Without it we sometimes have to resort to lazy_static. If the community is interested I can help pushing this forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.