Windows: don't error if access_mode is set on OpenOptions - #162984
Conversation
|
r? @clarfonthey rustbot has assigned @clarfonthey. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
0da2a41 to
ed4fe76
Compare
|
So, just looking at this code, I'm a little confused why the existing code doesn't already work, since the Branches are followed in order, and the other branches even explicitly mention that you need |
|
Like, for reviewability purposes, it would be helpful if you left everything in the previous match format at least for one commit so I can more easily tell what the difference is, then you can add a second commit refactoring out the |
|
@rustbot author |
|
I've added an intermediate commit so that the diff is more readable. Moving the |
ed4fe76 to
39f76c9
Compare
|
@rustbot ready |
|
Okay, yes, this is much easier to understand now. This looks good to me, especially with the motivation provided. @bors try jobs=x86_64-msvc-* (just because I keep getting burned by this…) |
This comment has been minimized.
This comment has been minimized.
Windows: don't error if `access_mode` is set on `OpenOptions` try-job: x86_64-msvc-*
|
💔 Test for 3f2aae8 failed: CI. Failed job:
|
|
A job failed! Check out the build log: (web) (plain enhanced) (plain) Click to see the possible cause of the failure (guessed by this bot) |
|
I do test locally but fair enough. Btw, job names have changed @bors try jobs=test-x86_64-msvc-* |
This comment has been minimized.
This comment has been minimized.
Windows: don't error if `access_mode` is set on `OpenOptions` try-job: test-x86_64-msvc-*
|
@bors try jobs=test-x86_64-msvc-* 🙃 |
This comment has been minimized.
This comment has been minimized.
Windows: don't error if `access_mode` is set on `OpenOptions` try-job: test-x86_64-msvc-*
|
@bors r+ rollup Thanks as always, just superstitious about Windows PRs randomly failing at this point :p |
…fonthey
Windows: don't error if `access_mode` is set on `OpenOptions`
In [RFC 1252] it was decided to artificially restrict the combination of `File` access modes and creation modes for the sake of cross-platform compatibility. This is tested in [`library/std/src/fs/tests.rs`]. As the RFC says, using platform specific function should allow overriding this.
On Windows, [`access_mode`] does indeed override `read`, `write` and `append`. However currently `OpenOptions` can still error if you don't use `read`, `write` or `append`. For example:
```rust
use std::os::windows::fs::OpenOptionsExt;
File::options()
.create_new(true)
.write(true) // useless except it prevents std from erroring
.access_mode(0)
```
This PR makes it so you don't need the misleading `write(true)`.
I also cleaned up the error handling for the access mode a bit. It was unnecessarily split between two functions.
[RFC 1252]: https://rust-lang.github.io/rfcs/1252-open-options.html#combination-of-access-modes-and-creation-modes
[`library/std/src/fs/tests.rs`]: https://github.com/rust-lang/rust/blob/420ed2a0c3d7225b1744266fd884d431b4d8cfe0/library/std/src/fs/tests.rs#L1517
[`access_mode`]: https://doc.rust-lang.org/stable/std/os/windows/fs/trait.OpenOptionsExt.html#tymethod.access_mode
Rollup of 15 pull requests Successful merges: - #161051 (When error from local macro, include macro def span) - #162750 (add case mapping fast paths for Latin-1) - #162831 (Do not continue past `rustc_resolve` when encountering duplicated items) - #162835 (rustdoc: account for nested parens and split text events in bare urls lint) - #163044 (Report runtime range endpoints for runtime values) - #163062 (Use x30 register name with LLVM 23+) - #158102 (When compiling without a specified `--edition`, emit a note) - #162984 (Windows: don't error if `access_mode` is set on `OpenOptions`) - #163005 (Avoid unreachable integer underflow check in `CStr::count_bytes()`) - #163019 (Prepare for the introduction of forced keywords (`k#`)) - #163020 (Dir: fix fallback impl for remove_dir) - #163047 (Use verbose suggestion for `mut binding` instead of `&mut binding`) - #163075 (Fix ArgAttributes mismatches in ABI UI tests for LoongArch64 and RiscV64) - #163079 (enable `f128` from `u64`/`i64` test) - #163082 (Remove `TypeChecker::root_cx`)
…fonthey
Windows: don't error if `access_mode` is set on `OpenOptions`
In [RFC 1252] it was decided to artificially restrict the combination of `File` access modes and creation modes for the sake of cross-platform compatibility. This is tested in [`library/std/src/fs/tests.rs`]. As the RFC says, using platform specific function should allow overriding this.
On Windows, [`access_mode`] does indeed override `read`, `write` and `append`. However currently `OpenOptions` can still error if you don't use `read`, `write` or `append`. For example:
```rust
use std::os::windows::fs::OpenOptionsExt;
File::options()
.create_new(true)
.write(true) // useless except it prevents std from erroring
.access_mode(0)
```
This PR makes it so you don't need the misleading `write(true)`.
I also cleaned up the error handling for the access mode a bit. It was unnecessarily split between two functions.
[RFC 1252]: https://rust-lang.github.io/rfcs/1252-open-options.html#combination-of-access-modes-and-creation-modes
[`library/std/src/fs/tests.rs`]: https://github.com/rust-lang/rust/blob/420ed2a0c3d7225b1744266fd884d431b4d8cfe0/library/std/src/fs/tests.rs#L1517
[`access_mode`]: https://doc.rust-lang.org/stable/std/os/windows/fs/trait.OpenOptionsExt.html#tymethod.access_mode
…uwer Rollup of 18 pull requests Successful merges: - #161051 (When error from local macro, include macro def span) - #161629 (Streamline `StateDiffCollector`) - #162750 (add case mapping fast paths for Latin-1) - #162831 (Do not continue past `rustc_resolve` when encountering duplicated items) - #162835 (rustdoc: account for nested parens and split text events in bare urls lint) - #162952 (Depend on lockfiles to prevent GC of the current session) - #163044 (Report runtime range endpoints for runtime values) - #163062 (Use x30 register name with LLVM 23+) - #158102 (When compiling without a specified `--edition`, emit a note) - #162939 (Declare multi-kind constants for MacroKinds) - #162984 (Windows: don't error if `access_mode` is set on `OpenOptions`) - #163005 (Avoid unreachable integer underflow check in `CStr::count_bytes()`) - #163019 (Prepare for the introduction of forced keywords (`k#`)) - #163020 (Dir: fix fallback impl for remove_dir) - #163047 (Use verbose suggestion for `mut binding` instead of `&mut binding`) - #163075 (Fix ArgAttributes mismatches in ABI UI tests for LoongArch64 and RiscV64) - #163079 (enable `f128` from `u64`/`i64` test) - #163082 (Remove `TypeChecker::root_cx`)
…uwer Rollup of 19 pull requests Successful merges: - #161051 (When error from local macro, include macro def span) - #161629 (Streamline `StateDiffCollector`) - #162750 (add case mapping fast paths for Latin-1) - #162831 (Do not continue past `rustc_resolve` when encountering duplicated items) - #162835 (rustdoc: account for nested parens and split text events in bare urls lint) - #162952 (Depend on lockfiles to prevent GC of the current session) - #163044 (Report runtime range endpoints for runtime values) - #163062 (Use x30 register name with LLVM 23+) - #163091 (miri subtree update) - #162939 (Declare multi-kind constants for MacroKinds) - #162984 (Windows: don't error if `access_mode` is set on `OpenOptions`) - #163005 (Avoid unreachable integer underflow check in `CStr::count_bytes()`) - #163019 (Prepare for the introduction of forced keywords (`k#`)) - #163020 (Dir: fix fallback impl for remove_dir) - #163041 (Be more explicit on suggestion type without changing how they are rendered) - #163047 (Use verbose suggestion for `mut binding` instead of `&mut binding`) - #163075 (Fix ArgAttributes mismatches in ABI UI tests for LoongArch64 and RiscV64) - #163079 (enable `f128` from `u64`/`i64` test) - #163082 (Remove `TypeChecker::root_cx`)
Rollup merge of #162984 - ChrisDenton:access-allowed, r=clarfonthey Windows: don't error if `access_mode` is set on `OpenOptions` In [RFC 1252] it was decided to artificially restrict the combination of `File` access modes and creation modes for the sake of cross-platform compatibility. This is tested in [`library/std/src/fs/tests.rs`]. As the RFC says, using platform specific function should allow overriding this. On Windows, [`access_mode`] does indeed override `read`, `write` and `append`. However currently `OpenOptions` can still error if you don't use `read`, `write` or `append`. For example: ```rust use std::os::windows::fs::OpenOptionsExt; File::options() .create_new(true) .write(true) // useless except it prevents std from erroring .access_mode(0) ``` This PR makes it so you don't need the misleading `write(true)`. I also cleaned up the error handling for the access mode a bit. It was unnecessarily split between two functions. [RFC 1252]: https://rust-lang.github.io/rfcs/1252-open-options.html#combination-of-access-modes-and-creation-modes [`library/std/src/fs/tests.rs`]: https://github.com/rust-lang/rust/blob/420ed2a0c3d7225b1744266fd884d431b4d8cfe0/library/std/src/fs/tests.rs#L1517 [`access_mode`]: https://doc.rust-lang.org/stable/std/os/windows/fs/trait.OpenOptionsExt.html#tymethod.access_mode
…uwer Rollup of 19 pull requests Successful merges: - rust-lang/rust#161051 (When error from local macro, include macro def span) - rust-lang/rust#161629 (Streamline `StateDiffCollector`) - rust-lang/rust#162750 (add case mapping fast paths for Latin-1) - rust-lang/rust#162831 (Do not continue past `rustc_resolve` when encountering duplicated items) - rust-lang/rust#162835 (rustdoc: account for nested parens and split text events in bare urls lint) - rust-lang/rust#162952 (Depend on lockfiles to prevent GC of the current session) - rust-lang/rust#163044 (Report runtime range endpoints for runtime values) - rust-lang/rust#163062 (Use x30 register name with LLVM 23+) - rust-lang/rust#163091 (miri subtree update) - rust-lang/rust#162939 (Declare multi-kind constants for MacroKinds) - rust-lang/rust#162984 (Windows: don't error if `access_mode` is set on `OpenOptions`) - rust-lang/rust#163005 (Avoid unreachable integer underflow check in `CStr::count_bytes()`) - rust-lang/rust#163019 (Prepare for the introduction of forced keywords (`k#`)) - rust-lang/rust#163020 (Dir: fix fallback impl for remove_dir) - rust-lang/rust#163041 (Be more explicit on suggestion type without changing how they are rendered) - rust-lang/rust#163047 (Use verbose suggestion for `mut binding` instead of `&mut binding`) - rust-lang/rust#163075 (Fix ArgAttributes mismatches in ABI UI tests for LoongArch64 and RiscV64) - rust-lang/rust#163079 (enable `f128` from `u64`/`i64` test) - rust-lang/rust#163082 (Remove `TypeChecker::root_cx`)
In RFC 1252 it was decided to artificially restrict the combination of
Fileaccess modes and creation modes for the sake of cross-platform compatibility. This is tested inlibrary/std/src/fs/tests.rs. As the RFC says, using platform specific function should allow overriding this.On Windows,
access_modedoes indeed overrideread,writeandappend. However currentlyOpenOptionscan still error if you don't useread,writeorappend. For example:This PR makes it so you don't need the misleading
write(true).I also cleaned up the error handling for the access mode a bit. It was unnecessarily split between two functions.