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 std::fs::write documentation precision #98662

Merged
merged 1 commit into from
Jul 16, 2022

Conversation

LucasDumont
Copy link
Contributor

@LucasDumont LucasDumont commented Jun 29, 2022

Fixes #97947.

As mentioned in #97947, the documentation is updated

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jun 29, 2022
@rustbot
Copy link
Collaborator

rustbot commented Jun 29, 2022

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then
please comment with @rustbot label +T-libs-api -T-libs to tag it
appropriately. If this PR contains changes to any unstable APIs please edit
the PR description to add a link to the relevant API Change
Proposal

or create one
if you haven't already. If you're unsure where your change falls no worries,
just leave it as is and the reviewer will take a look and make a decision to
forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing
    stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding
    permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rust-highfive
Copy link
Collaborator

r? @thomcc

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 29, 2022
@@ -295,6 +295,9 @@ pub fn read_to_string<P: AsRef<Path>>(path: P) -> io::Result<String> {
/// This function will create a file if it does not exist,
/// and will entirely replace its contents if it does.
///
/// However, if a parent directory in the path doesn't exist,
/// the function will fail.
Copy link
Member

Choose a reason for hiding this comment

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

I think this can be simplified by folding it into the previous paragraph. Something along the lines of

[...] will create a file if it does not exist - but only if the directory itself exists - and will [...]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, but if we take what @thomcc said into account and introduce hypothetical platforms, this will become hard to fold in the previous paragraph without hindering the readability.

@thomcc
Copy link
Member

thomcc commented Jun 29, 2022

Do we care at all about any hypothetical platforms where this isn't the case?

@thomcc
Copy link
Member

thomcc commented Jun 30, 2022

I think this counts as a new guarantee. My only hesitation here is that while this is extremely reasonable and guaranteed on all current platforms (I think?), I think it also would not really be possible for us to implement this guarantee inside the stdlib, without race conditions.

Not really sure what to do about this, though.

@rustbot label +T-libs-api -T-libs

@rustbot rustbot added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 30, 2022
@the8472
Copy link
Member

the8472 commented Jun 30, 2022

You can always water it down with a "might not" or something like that.

@ChrisDenton
Copy link
Member

As this is documented to be a wrapper around File::create and write_all, should File::create also have this caveat added?

@thomcc
Copy link
Member

thomcc commented Jul 1, 2022

If we aren't guaranteeing this (e.g. say "might not"), then yeah, I don't think this needs API sign off.

So, can you reduce the guarantee to "might not" (as suggested by #98662 (comment)), and also, add a similar comment to File::create (as suggested by #98662 (comment))?

library/std/src/fs.rs Outdated Show resolved Hide resolved
library/std/src/fs.rs Outdated Show resolved Hide resolved
As mentioned in rust-lang#97947, the documentation is updated
@thomcc
Copy link
Member

thomcc commented Jul 16, 2022

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 16, 2022

📌 Commit 07a0fd2 has been approved by thomcc

It is now in the queue for this repository.

@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 Jul 16, 2022
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jul 16, 2022
…omcc

Add std::fs::write documentation precision

Fixes rust-lang#97947.

As mentioned in rust-lang#97947, the documentation is updated
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 16, 2022
Rollup of 7 pull requests

Successful merges:

 - rust-lang#98387 (Add new unstable API `downcast` to `std::io::Error`)
 - rust-lang#98662 (Add std::fs::write documentation precision)
 - rust-lang#99253 (Remove FIXME from MIR `always_storage_live_locals`)
 - rust-lang#99264 (Fix typo in mod.rs)
 - rust-lang#99270 (Add `#[must_use]` to `Box::from_raw`)
 - rust-lang#99277 (Stabilize `core::ffi::CStr`, `alloc::ffi::CString`, and friends)
 - rust-lang#99307 (Add regression test for rust-lang#64401)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 06eb90e into rust-lang:master Jul 16, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 16, 2022
@LucasDumont LucasDumont deleted the document_fs_write branch July 20, 2022 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

std::fs::write should document that it doesn't create directories
7 participants