feat: Implement cast typecheck and diagnostics #17984
Conversation
rustbot
added
the
S-waiting-on-review
ShoyuVanilla
force-pushed
the
cast
branch
5 times, most recently
from
32f0175 to
35895fb
Compare
crates/hir-ty/src/infer/cast.rs
Outdated
| m_cast: Mutability, | ||
| table: &mut InferenceTable<'_>, | ||
| ) -> Result<(), CastError> { | ||
| // Mutability order is opposite tu rustc. `Mut < Not` |
There was a problem hiding this comment.
Might be good to change that on our side then (separately from this), the less we diverge from rustc with things the better we can draw comparisons between the code bases (which imo for type check is quite important)
There was a problem hiding this comment.
I agree. But this type is from chalk_ir. Maybe we should change this at there?
There was a problem hiding this comment.
BTW, another typo "tu" here 😅
There was a problem hiding this comment.
Ah I see, probably no point in changing it then. But we should keep that in mind for the trait solver migration 😬
There was a problem hiding this comment.
Yes, there are many places in our code that tailored for chalk quirks and I can remember some of them. The ones that no one remembers or knows would be covered by our tests, I hope 😨
|
I was blocked on mir evaluation failures when I push cast adjustments into InferenceResult and finally found the reason 🥳 our mir castkind is doing both mir and hir_typeck parts of rustc and thus pushing adjustment makes erroneous duplication. |
|
So, this will also fix #16564 when finished |
| @@ -278,7 +282,7 @@ fn main() { | |||
| "#, | |||
| r#" | |||
| unsafe fn func() { | |||
| let x = &5 as *const usize; | |||
| let x = &5_usize as *const usize; | |||
There was a problem hiding this comment.
Previous cast &5 as *const usize is illegal
| @@ -399,7 +399,7 @@ extern "C" { | |||
| fn memcmp(s1: *const u8, s2: *const u8, n: usize) -> i32; | |||
| } | |||
|
|
|||
| fn my_cmp(x: &[u8], y: &[u8]) -> i32 { | |||
| fn my_cmp(x: &[u8; 3], y: &[u8; 3]) -> i32 { | |||
There was a problem hiding this comment.
Previous cast &[u8] as *const u8 is invalid in rustc
| // Structs can have DST as its last field and such cases are not handled | ||
| // as unsized by the chalk, so we do this manually |
There was a problem hiding this comment.
(not sure how relevant this is but) this reminds me, we do not populate chalk's AdtvariantDatum's fields right now as that slows down things by quite a bit. Just letting you know so you are aware (the relevant part for that is stubbed out at the moment even though we have it implemented)
rust-analyzer/crates/hir-ty/src/chalk_db.rs
Lines 768 to 783 in 134e0a4
There was a problem hiding this comment.
Oh, I was wondering why chalk is saying struct Foo { inner: [u8] } is Sized and that was the answer😄
There was a problem hiding this comment.
iirc it added like 10 seconds to inference on self which is a bit much + at the time we couldnt inspect third party deps of the standard library (which we soon can) so we could reconsider if this turns out to cause troubles
There was a problem hiding this comment.
In that case we might omit it and consider it as sized; that will make some false negative cast errors but we'll not have irritating false positives anyway (though we should change function name into somewhat like maybe_sized to avoid possible footguns in future that might be caused by trusting that function will always return the correct value 😅 )
|
Thanks! |
|
☀️ Test successful - checks-actions |
Fixes #17897 and fixes #16564
Mainly adopted from https://github.com/rust-lang/rust/blob/100fde5246bf56f22fb5cc85374dd841296fce0e/compiler/rustc_hir_typeck/src/cast.rs