-
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
unsafeck: Don't treat AscribeUserType as use #80826
Conversation
Previously, if the MIR had an AscribeUserType statement that ascribed a type to the pointee of a raw pointer, it would be treated as a dereference of the raw pointer for purposes of unsafe-checking. For example, the following code would be treated as containing a raw-pointer dereference: fn foo(ptr: *const bool) { let _: bool = *ptr; } Producing this error: error[E0133]: dereference of raw pointer is unsafe and requires unsafe function or block --> issue-80059.rs:2:12 | 2 | let _: bool = *ptr; | ^^^^ dereference of raw pointer Note that the error points to the type ascription as having a dereference! That's because the generated AscribeUserType MIR statement is treated as containing a dereference of `_1`: AscribeUserType((*_1), +, UserTypeProjection { base: UserType(1), projs: [] }); Now the unsafe-checker ignores uses inside `AscribeUserType` statements, which means this code now compiles successfully. ----- Note that this code: fn foo(ptr: *const bool) { let _: bool = *ptr; } does *not* produce an error (it compiles fine) because of the magical behavior of the `_` (wildcard) pattern (see rust-lang#80059).
I'm not sure if the old behavior was intended or not – it doesn't seem right, but So do we need T-lang approval? |
Note that this code: fn foo(ptr: *const bool) {
let _: bool = { *ptr };
} still (rightly – #80059 (comment)) produces an error:
so the behavior wrt unsafe-checking of |
FWIW, I am not sure I agree with the intent of this PR, because...
... now the two behaviors are the same, but arguably, they are both questionable. Is it truly better to be consistent here? I am not convinced it is worth changing this until we have t-lang consensus for what the intended behavior is. Once that consensus exists, surely the case with and without type ascription should behave the same. But until then, it is unclear which of them is wrong. |
I agree with your reasoning, but I still think we should make this change. A type ascription (to my knowledge) does not count as a "use" of a value, regardless of whether |
That's fair, I'd draw a different conclusion though. ;) I'm not an expert on the unsafety checker anyway, so maybe someone else can review this... |
i think T-lang discussed this and ended up agreeing with Ralf, but I want to go review the footage before I close it based on a memory, especially late at night... |
https://youtu.be/4dRVeTn5-jQ?t=1398 is the lang team meeting I was thinking of. The essential outcome is that we agreed on these things:
So I'm going to close this PR. Sorry for taking so long to get around to that double-checking of the lang team meeting. |
Once unsafeck operates on the THIR (cc rust-lang/compiler-team#402), I'm assuming the behavior will be consistent between |
Part of #80059.
Previously, if the MIR had an AscribeUserType statement that ascribed a
type to the pointee of a raw pointer, it would be treated as a dereference
of the raw pointer for purposes of unsafe-checking. For example, the
following code would be treated as containing a raw-pointer dereference:
Producing this error:
Note that the error points to the type ascription as having a
dereference! That's because the generated AscribeUserType MIR statement
is treated as containing a dereference of
_1
:Now the unsafe-checker ignores uses inside
AscribeUserType
statements,which means this code now compiles successfully.
Note that this code:
does not produce an error (it compiles fine) because of the magical
behavior of the
_
(wildcard) pattern (see #80059).r? @RalfJung