-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Rollup of 3 pull requests #129234
Closed
Closed
Rollup of 3 pull requests #129234
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For following: ```rust struct A; impl A { fn test4(&self) { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method } ``` Suggest: ```rust impl A { fn test4(&self) -> Result<(), Box<dyn std::error::Error>> { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method Ok(()) } } ``` For rust-lang#125997
... by using `std::fs::remove_dir_all`, which handles a bunch of edge cases including read-only files and symlinks which are extremely tricky on Windows.
…,jieyouxu Stabilize `raw_ref_op` (RFC 2582) This stabilizes the syntax `&raw const $expr` and `&raw mut $expr`. It has existed unstably for ~4 years now, and has been exposed on stable via the `addr_of` and `addr_of_mut` macros since Rust 1.51 (released more than 3 years ago). I think it has become clear that these operations are here to stay. So it is about time we give them proper primitive syntax. This has two advantages over the macro: - Being macros, `addr_of`/`addr_of_mut` could in theory do arbitrary magic with the expression on which they work. The only "magic" they actually do is using the argument as a place expression rather than as a value expression. Place expressions are already a subtle topic and poorly understood by many programmers; having this hidden behind a macro using unstable language features makes this even worse. Conversely, people do have an idea of what happens below `&`/`&mut`, so we can make the subtle topic a lot more approachable by connecting to existing intuition. - The name `addr_of` is quite unfortunate from today's perspective, given that we have accepted provenance as a reality, which means that a pointer is *not* just an address. Strict provenance has a method, `addr`, which extracts the address of a pointer; using the term `addr` in two different ways is quite unfortunate. That's why this PR soft-deprecates `addr_of` -- we will wait a long time before actually showing any warning here, but we should start telling people that the "addr" part of this name is somewhat misleading, and `&raw` avoids that potential confusion. In summary, this syntax improves developers' ability to conceptualize the operational semantics of Rust, while making a fundamental operation frequently used in unsafe code feel properly built in. Possible questions to consider, based on the RFC and [this](rust-lang#64490 (comment)) great summary by `@CAD97:` - Some questions are entirely about the semantics. The semantics are the same as with the macros so I don't think this should have any impact on this syntax PR. Still, for completeness' sake: - Should `&raw const *mut_ref` give a read-only pointer? - Tracked at: rust-lang/unsafe-code-guidelines#257 - I think ideally the answer is "no". Stacked Borrows says that pointer is read-only, but Tree Borrows says it is mutable. - What exactly does `&raw const (*ptr).field` require? Answered in [the reference](https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html): the arithmetic to compute the field offset follows the rules of `ptr::offset`, making it UB if it goes out-of-bounds. Making this a safe operation (using `wrapping_offset` rules) is considered too much of a loss for alias analysis. - Choose a different syntax? I don't want to re-litigate the RFC. The only credible alternative that has been proposed is `&raw $place` instead of `&raw const $place`, which (IIUC) could be achieved by making `raw` a contextual keyword in a new edition. The type is named `*const T`, so the explicit `const` is consistent in that regard. `&raw expr` lacks the explicit indication of immutability. However, `&raw const expr` is quite a but longer than `addr_of!(expr)`. - Shouldn't we have a completely new, better raw pointer type instead? Yes we all want to see that happen -- but I don't think we should block stabilization on that, given that such a nicer type is not on the horizon currently and given the issues with `addr_of!` mentioned above. (If we keep the `&raw $place` syntax free for this, we could use it in the future for that new type.) - What about the lint the RFC talked about? It hasn't been implemented yet. Given that the problematic code is UB with or without this stabilization, I don't think the lack of the lint should block stabilization. - I created an issue to track adding it: rust-lang#127724 - Other points from the "future possibilites of the RFC - "Syntactic sugar" extension: this has not been implemented. I'd argue this is too confusing, we should stick to what the RFC suggested and if we want to do anything about such expressions, add the lint. - Encouraging / requiring `&raw` in situations where references are often/definitely incorrect: this has been / is being implemented. On packed fields this already is a hard error, and for `static mut` a lint suggesting raw pointers is being rolled out. - Lowering of casts: this has been implemented. (It's also an invisible implementation detail.) - `offsetof` woes: we now have native `offset_of` so this is not relevant any more. To be done before landing: - [x] Suppress `unused_parens` lint around `&raw {const|mut}` expressions - See bottom of rust-lang#127679 (comment) for rationale - Implementation: rust-lang#128782 - [ ] Update the Reference. - rust-lang/reference#1567 Fixes rust-lang#64490 cc `@rust-lang/lang` `@rust-lang/opsem` try-job: x86_64-msvc try-job: test-various try-job: dist-various-1 try-job: armhf-gnu try-job: aarch64-apple
Suggest adding Result return type for associated method in E0277. Recommit rust-lang#126515 because I messed up during rebase, Suggest adding Result return type for associated method in E0277. For following: ```rust struct A; impl A { fn test4(&self) { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method } ``` Suggest: ```rust impl A { fn test4(&self) -> Result<(), Box<dyn std::error::Error>> { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method Ok(()) } } ``` For rust-lang#125997 r? `@cjgillot`
…links, r=Kobzol bootstrap: fix clean's remove_dir_all implementation It turns out bootstrap's `clean.rs`'s hand-rolled `rm_rf` (which probably comes before `std::fs::remove_dir_all` was stable) is very broken on native Windows around both read-only files/directories and especially symbolic links. So instead of rolling our own, just use `std::fs::remove_dir_all`. This is a blocker for compiletest's own `rm_rf` implementation rust-lang#129155 which happens to be also buggy, which in turn is a blocker for the rmake.rs test port rust-lang#128562 that heavily exercises symlinks (I was reviewing rust-lang#128562 and testing it on native Windows which is how I found out). I also left a FIXME for `detect_src_and_out` due to a failing assertion on native Windows (opened rust-lang#129188): ``` ---- core::config::tests::detect_src_and_out stdout ---- thread 'core::config::tests::detect_src_and_out' panicked at src\core\config\tests.rs:72:13: assertion `left == right` failed left: "E:\\tmp" right: "C:\\tmp" ``` Fixes rust-lang#112544 (because now we handle Windows symlinks properly). try-job: x86_64-msvc try-job: i686-mingw try-job: test-various try-job: armhf-gnu try-job: aarch64-apple try-job: aarch64-gnu
rustbot
added
PG-exploit-mitigations
Project group: Exploit mitigations
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
T-bootstrap
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
T-libs
Relevant to the library team, which will review and decide on the PR/issue.
rollup
A PR which is a rollup
labels
Aug 18, 2024
@bors r+ rollup=never p=3 |
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
Aug 18, 2024
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 18, 2024
Rollup of 3 pull requests Successful merges: - rust-lang#127679 (Stabilize `raw_ref_op` (RFC 2582)) - rust-lang#128084 (Suggest adding Result return type for associated method in E0277.) - rust-lang#129187 (bootstrap: fix clean's remove_dir_all implementation) r? `@ghost` `@rustbot` modify labels: rollup
wait this has merge conflicts |
bors
added
S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
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-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
Aug 18, 2024
☔ The latest upstream changes (presumably #129230) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
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-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
labels
Aug 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
PG-exploit-mitigations
Project group: Exploit mitigations
rollup
A PR which is a rollup
S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
T-bootstrap
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
T-libs
Relevant to the library team, which will review and decide on the PR/issue.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Successful merges:
raw_ref_op
(RFC 2582) #127679 (Stabilizeraw_ref_op
(RFC 2582))r? @ghost
@rustbot modify labels: rollup
Create a similar rollup