-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Implement MIR lowering for unsafe binders #130514
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #130724) made this pull request unmergeable. Please resolve the merge conflicts. |
3aa766a
to
ce3fbcb
Compare
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #127117) made this pull request unmergeable. Please resolve the merge conflicts. |
…, r=oli-obk Add AST support for unsafe binders I'm splitting up rust-lang#130514 into pieces. It's impossible for me to keep up with a huge PR like that. I'll land type system support for this next, probably w/o MIR lowering, which will come later. r? `@oli-obk` cc `@BoxyUwU` and `@lcnr` who also may want to look at this, though this PR doesn't do too much yet
Rollup merge of rust-lang#134140 - compiler-errors:unsafe-binders-ast, r=oli-obk Add AST support for unsafe binders I'm splitting up rust-lang#130514 into pieces. It's impossible for me to keep up with a huge PR like that. I'll land type system support for this next, probably w/o MIR lowering, which will come later. r? `@oli-obk` cc `@BoxyUwU` and `@lcnr` who also may want to look at this, though this PR doesn't do too much yet
ce3fbcb
to
fab3984
Compare
This comment has been minimized.
This comment has been minimized.
…, r=oli-obk Add AST support for unsafe binders I'm splitting up rust-lang#130514 into pieces. It's impossible for me to keep up with a huge PR like that. I'll land type system support for this next, probably w/o MIR lowering, which will come later. r? `@oli-obk` cc `@BoxyUwU` and `@lcnr` who also may want to look at this, though this PR doesn't do too much yet
☔ The latest upstream changes (presumably #134414) made this pull request unmergeable. Please resolve the merge conflicts. |
9c432eb
to
b0f3844
Compare
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #134326) made this pull request unmergeable. Please resolve the merge conflicts. |
bfe01d2
to
50d4440
Compare
@@ -0,0 +1,41 @@ | |||
//@ known-bug: unknown |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test will (after I add a new trait instead of using Copy
) demonstrate that we reason about moves correctly with unsafe binders.
@@ -66,6 +66,10 @@ impl<'tcx> Iterator for Prefixes<'tcx> { | |||
self.next = Some(cursor_base); | |||
return Some(cursor); | |||
} | |||
ProjectionElem::UnwrapUnsafeBinder(_) => { | |||
self.next = Some(cursor_base); | |||
return Some(cursor); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just roll over unwrapunsafebinder just like with opaquecast?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the contrary, why is it valid to ignore this projection elem? I want to make sure I'm visiting it when tracking moves, right? Or am I misunderstanding how these prefixes are used?
In my brain, unsafe binders are equivalent to structs with a single field, so we should be treating this much like a field elem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are just used for improving diagnostics, to be able to inform ppl when to split their borrows. It is useful to handle field accesses here, because borrowck can understand that any projection that goes through a field does not conflict with a projection that goes through another field. I unfortunately can't figure out how to make it care about the unsafe wrapper at all, so... 🤷 let's go with this and figure things out when we actually are using unsafe binders a lot and see some real world examples and conflicts.
3700fe8
to
442b9a9
Compare
@bors r+ |
…oli-obk Implement MIR lowering for unsafe binders This is the final bit of the unsafe binders puzzle. It implements MIR, CTFE, and codegen for unsafe binders, and enforces that (for now) they are `Copy`. Later on, I'll introduce a new trait that relaxes this requirement to being "is `Copy` or `ManuallyDrop<T>`" which more closely models how we treat union fields. Namely, wrapping unsafe binders is now `Rvalue::WrapUnsafeBinder`, which acts much like an `Rvalue::Aggregate`. Unwrapping unsafe binders are implemented as a MIR projection `ProjectionElem::UnwrapUnsafeBinder`, which acts much like `ProjectionElem::Field`. Tracking: - rust-lang#130516
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#130514 (Implement MIR lowering for unsafe binders) - rust-lang#135684 (docs: Documented Send and Sync requirements for Mutex + MutexGuard) - rust-lang#135760 (Add `unchecked_disjoint_bitor` per ACP373) - rust-lang#136154 (Use +secure-plt for powerpc-unknown-linux-gnu{,spe}) - rust-lang#136309 (set rustc dylib on manually constructed rustc command) - rust-lang#136339 (CompileTest: Add Directives to Ignore `arm-unknown-*` Targets) - rust-lang#136368 (Make comma separated lists of anything easier to make for errors) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#130514 (Implement MIR lowering for unsafe binders) - rust-lang#135684 (docs: Documented Send and Sync requirements for Mutex + MutexGuard) - rust-lang#136307 (Implement all mix/max functions in a (hopefully) more optimization amendable way) - rust-lang#136360 (Stabilize `once_wait`) - rust-lang#136364 (document that ptr cmp is unsigned) - rust-lang#136374 (Add link attribute for Enzyme's LLVMRust FFI) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#130514 (Implement MIR lowering for unsafe binders) - rust-lang#135684 (docs: Documented Send and Sync requirements for Mutex + MutexGuard) - rust-lang#136307 (Implement all mix/max functions in a (hopefully) more optimization amendable way) - rust-lang#136360 (Stabilize `once_wait`) - rust-lang#136364 (document that ptr cmp is unsigned) - rust-lang#136374 (Add link attribute for Enzyme's LLVMRust FFI) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#130514 - compiler-errors:unsafe-binders, r=oli-obk Implement MIR lowering for unsafe binders This is the final bit of the unsafe binders puzzle. It implements MIR, CTFE, and codegen for unsafe binders, and enforces that (for now) they are `Copy`. Later on, I'll introduce a new trait that relaxes this requirement to being "is `Copy` or `ManuallyDrop<T>`" which more closely models how we treat union fields. Namely, wrapping unsafe binders is now `Rvalue::WrapUnsafeBinder`, which acts much like an `Rvalue::Aggregate`. Unwrapping unsafe binders are implemented as a MIR projection `ProjectionElem::UnwrapUnsafeBinder`, which acts much like `ProjectionElem::Field`. Tracking: - rust-lang#130516
…oli-obk Implement MIR lowering for unsafe binders This is the final bit of the unsafe binders puzzle. It implements MIR, CTFE, and codegen for unsafe binders, and enforces that (for now) they are `Copy`. Later on, I'll introduce a new trait that relaxes this requirement to being "is `Copy` or `ManuallyDrop<T>`" which more closely models how we treat union fields. Namely, wrapping unsafe binders is now `Rvalue::WrapUnsafeBinder`, which acts much like an `Rvalue::Aggregate`. Unwrapping unsafe binders are implemented as a MIR projection `ProjectionElem::UnwrapUnsafeBinder`, which acts much like `ProjectionElem::Field`. Tracking: - rust-lang#130516
…oli-obk Implement MIR lowering for unsafe binders This is the final bit of the unsafe binders puzzle. It implements MIR, CTFE, and codegen for unsafe binders, and enforces that (for now) they are `Copy`. Later on, I'll introduce a new trait that relaxes this requirement to being "is `Copy` or `ManuallyDrop<T>`" which more closely models how we treat union fields. Namely, wrapping unsafe binders is now `Rvalue::WrapUnsafeBinder`, which acts much like an `Rvalue::Aggregate`. Unwrapping unsafe binders are implemented as a MIR projection `ProjectionElem::UnwrapUnsafeBinder`, which acts much like `ProjectionElem::Field`. Tracking: - rust-lang#130516
- unsafe binders, c.f. rust-lang/rust#130514
- unsafe binders, c.f. rust-lang/rust#130514
Upgrade toolchain to 2/10. I **highly recommend** reviewing this PR commit-by-commit. The description in each commit message links to the upstream PRs that prompted those particular changes. ## Callouts - 2/1 had a lot of formatting changes. I split the commits for that day into formatting changes and functionality changes accordingly. - 2/5 introduced a regression in our delayed UB instrumentation, so I made a new fixme test. See #3881 for details. ## Culprit PRs: rust-lang/rust#134424 rust-lang/rust#130514 rust-lang/rust#135748 rust-lang/rust#136590 rust-lang/rust#135318 rust-lang/rust#135265 rust-lang/rust@bcb8565 rust-lang/rust#136471 rust-lang/rust#136645 Resolves #3863 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.
…, r=oli-obk Add AST support for unsafe binders I'm splitting up rust-lang#130514 into pieces. It's impossible for me to keep up with a huge PR like that. I'll land type system support for this next, probably w/o MIR lowering, which will come later. r? `@oli-obk` cc `@BoxyUwU` and `@lcnr` who also may want to look at this, though this PR doesn't do too much yet
This is the final bit of the unsafe binders puzzle. It implements MIR, CTFE, and codegen for unsafe binders, and enforces that (for now) they are
Copy
. Later on, I'll introduce a new trait that relaxes this requirement to being "isCopy
orManuallyDrop<T>
" which more closely models how we treat union fields.Namely, wrapping unsafe binders is now
Rvalue::WrapUnsafeBinder
, which acts much like anRvalue::Aggregate
. Unwrapping unsafe binders are implemented as a MIR projectionProjectionElem::UnwrapUnsafeBinder
, which acts much likeProjectionElem::Field
.Tracking: