feat: Implement cast typecheck and diagnostics #17984

ShoyuVanilla · 2024-08-28T15:13:50Z

Fixes #17897 and fixes #16564
Mainly adopted from https://github.com/rust-lang/rust/blob/100fde5246bf56f22fb5cc85374dd841296fce0e/compiler/rustc_hir_typeck/src/cast.rs

crates/hir-ty/src/infer/cast.rs

Veykril · 2024-08-30T08:46:18Z

crates/hir-ty/src/infer/cast.rs

+    m_cast: Mutability,
+    table: &mut InferenceTable<'_>,
+) -> Result<(), CastError> {
+    // Mutability order is opposite tu rustc. `Mut < Not`


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)

I agree. But this type is from chalk_ir. Maybe we should change this at there?

BTW, another typo "tu" here 😅

Ah I see, probably no point in changing it then. But we should keep that in mind for the trait solver migration 😬

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 😨

ShoyuVanilla · 2024-09-01T17:56:00Z

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.
I'll continue on this tommorrow

ShoyuVanilla · 2024-09-01T17:58:47Z

So, this will also fix #16564 when finished

ShoyuVanilla · 2024-09-02T18:40:41Z

crates/ide-diagnostics/src/handlers/missing_unsafe.rs

@@ -278,7 +282,7 @@ fn main() {
 "#,
            r#"
 unsafe fn func() {
-    let x = &5 as *const usize;
+    let x = &5_usize as *const usize;


Previous cast &5 as *const usize is illegal

ShoyuVanilla · 2024-09-02T18:50:02Z

crates/hir-ty/src/mir/eval/tests.rs

@@ -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 {


Previous cast &[u8] as *const u8 is invalid in rustc

crates/hir-def/src/data/adt.rs

Veykril · 2024-09-02T18:52:25Z

crates/hir-ty/src/infer/unify.rs

+                // Structs can have DST as its last field and such cases are not handled
+                // as unsized by the chalk, so we do this manually


(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

let _variant_id_to_fields = |id: VariantId| {

let variant_data = &id.variant_data(db.upcast());

let fields = if variant_data.fields().is_empty() {

vec![]

} else {

let field_types = db.field_types(id);

variant_data

.fields()

.iter()

.map(|(idx, _)| field_types[idx].clone().substitute(Interner, &bound_vars_subst))

.filter(|it| !it.contains_unknown())

.collect()

};

rust_ir::AdtVariantDatum { fields }

};

let variant_id_to_fields = |_: VariantId| rust_ir::AdtVariantDatum { fields: vec![] };

Oh, I was wondering why chalk is saying struct Foo { inner: [u8] } is Sized and that was the answer😄

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

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 😅 )

crates/hir-def/src/data/adt.rs

Veykril · 2024-09-03T05:46:05Z

Thanks!
@bors r+

bors · 2024-09-03T05:46:07Z

📌 Commit d186bdc has been approved by Veykril

It is now in the queue for this repository.

bors · 2024-09-03T06:00:13Z

⌛ Testing commit d186bdc with merge 6e84451...

bors · 2024-09-03T06:13:38Z

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 6e84451 to master...

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 28, 2024

ShoyuVanilla force-pushed the cast branch 5 times, most recently from 32f0175 to 35895fb Compare August 29, 2024 17:23

Veykril reviewed Aug 30, 2024

View reviewed changes

crates/hir-ty/src/infer/cast.rs Outdated Show resolved Hide resolved

Veykril reviewed Aug 30, 2024

View reviewed changes

ShoyuVanilla force-pushed the cast branch from 35895fb to d26b6fa Compare September 2, 2024 18:38

ShoyuVanilla marked this pull request as ready for review September 2, 2024 18:38

ShoyuVanilla commented Sep 2, 2024

View reviewed changes

Veykril reviewed Sep 2, 2024

View reviewed changes

feat: Implement cast typechecks

d186bdc

ShoyuVanilla force-pushed the cast branch from d26b6fa to d186bdc Compare September 2, 2024 19:11

ShoyuVanilla commented Sep 2, 2024

View reviewed changes

crates/hir-def/src/data/adt.rs Show resolved Hide resolved

Veykril approved these changes Sep 3, 2024

View reviewed changes

bors merged commit 6e84451 into rust-lang:master Sep 3, 2024
11 checks passed

ShoyuVanilla deleted the cast branch September 3, 2024 06:17

frxstrem mentioned this pull request Sep 4, 2024

False positive non-primitive cast error in recent nightly #18047

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Implement cast typecheck and diagnostics #17984

feat: Implement cast typecheck and diagnostics #17984

ShoyuVanilla commented Aug 28, 2024 •

edited

Loading

Veykril Aug 30, 2024

ShoyuVanilla Aug 30, 2024

ShoyuVanilla Aug 30, 2024

Veykril Aug 30, 2024

ShoyuVanilla Aug 30, 2024

ShoyuVanilla commented Sep 1, 2024

ShoyuVanilla commented Sep 1, 2024

ShoyuVanilla Sep 2, 2024

ShoyuVanilla Sep 2, 2024

Veykril Sep 2, 2024

ShoyuVanilla Sep 2, 2024

Veykril Sep 2, 2024

ShoyuVanilla Sep 2, 2024

Veykril commented Sep 3, 2024

bors commented Sep 3, 2024

bors commented Sep 3, 2024

bors commented Sep 3, 2024

		// Structs can have DST as its last field and such cases are not handled
		// as unsized by the chalk, so we do this manually

	let _variant_id_to_fields = \|id: VariantId\| {
	let variant_data = &id.variant_data(db.upcast());
	let fields = if variant_data.fields().is_empty() {
	vec![]
	} else {
	let field_types = db.field_types(id);
	variant_data
	.fields()
	.iter()
	.map(\|(idx, _)\| field_types[idx].clone().substitute(Interner, &bound_vars_subst))
	.filter(\|it\| !it.contains_unknown())
	.collect()
	};
	rust_ir::AdtVariantDatum { fields }
	};
	let variant_id_to_fields = \|_: VariantId\| rust_ir::AdtVariantDatum { fields: vec![] };

feat: Implement cast typecheck and diagnostics #17984

feat: Implement cast typecheck and diagnostics #17984

Conversation

ShoyuVanilla commented Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShoyuVanilla commented Sep 1, 2024

ShoyuVanilla commented Sep 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Veykril commented Sep 3, 2024

bors commented Sep 3, 2024

bors commented Sep 3, 2024

bors commented Sep 3, 2024

ShoyuVanilla commented Aug 28, 2024 •

edited

Loading