Skip to content

fix: Yet another false positive invalid cast diagnostic #19432

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 7, 2025
+134 −26
Merged

fix: Yet another false positive invalid cast diagnostic #19432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions crates/hir-ty/src/infer/cast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl CastTy {
None
}
}
TyKind::Raw(m, ty) => Some(Self::Ptr(table.resolve_ty_shallow(ty), *m)),
TyKind::Raw(m, ty) => Some(Self::Ptr(ty.clone(), *m)),
TyKind::Function(_) => Some(Self::FnPtr),
_ => None,
}
Expand Down Expand Up @@ -105,9 +105,8 @@ impl CastCheck {
F: FnMut(ExprId, Vec<Adjustment>),
G: FnMut(ExprId),
{
table.resolve_obligations_as_possible();
self.expr_ty = table.resolve_ty_shallow(&self.expr_ty);
self.cast_ty = table.resolve_ty_shallow(&self.cast_ty);
self.expr_ty = table.eagerly_normalize_and_resolve_shallow_in(self.expr_ty.clone());
self.cast_ty = table.eagerly_normalize_and_resolve_shallow_in(self.cast_ty.clone());

if self.expr_ty.contains_unknown() || self.cast_ty.contains_unknown() {
return Ok(());
Expand Down Expand Up @@ -153,7 +152,7 @@ impl CastCheck {
(None, Some(t_cast)) => match self.expr_ty.kind(Interner) {
TyKind::FnDef(..) => {
let sig = self.expr_ty.callable_sig(table.db).expect("FnDef had no sig");
let sig = table.normalize_associated_types_in(sig);
let sig = table.eagerly_normalize_and_resolve_shallow_in(sig);
let fn_ptr = TyKind::Function(sig.to_fn_ptr()).intern(Interner);
if let Ok((adj, _)) = table.coerce(&self.expr_ty, &fn_ptr, CoerceNever::Yes)
{
Expand All @@ -165,7 +164,6 @@ impl CastCheck {
(CastTy::FnPtr, t_cast)
}
TyKind::Ref(mutbl, _, inner_ty) => {
let inner_ty = table.resolve_ty_shallow(inner_ty);
return match t_cast {
CastTy::Int(_) | CastTy::Float => match inner_ty.kind(Interner) {
TyKind::Scalar(
Expand All @@ -180,13 +178,13 @@ impl CastCheck {
},
// array-ptr-cast
CastTy::Ptr(t, m) => {
let t = table.resolve_ty_shallow(&t);
let t = table.eagerly_normalize_and_resolve_shallow_in(t);
if !table.is_sized(&t) {
return Err(CastError::IllegalCast);
}
self.check_ref_cast(
table,
&inner_ty,
inner_ty,
*mutbl,
&t,
m,
Expand Down Expand Up @@ -359,7 +357,7 @@ impl CastCheck {
}
}

#[derive(PartialEq, Eq)]
#[derive(Debug, PartialEq, Eq)]
enum PointerKind {
// thin pointer
Thin,
Expand All @@ -373,8 +371,7 @@ enum PointerKind {
}

fn pointer_kind(ty: &Ty, table: &mut InferenceTable<'_>) -> Result<Option<PointerKind>, ()> {
let ty = table.resolve_ty_shallow(ty);
let ty = table.normalize_associated_types_in(ty);
let ty = table.eagerly_normalize_and_resolve_shallow_in(ty.clone());

if table.is_sized(&ty) {
return Ok(Some(PointerKind::Thin));
Expand Down
95 changes: 81 additions & 14 deletions crates/hir-ty/src/infer/unify.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,64 @@ impl<'a> InferenceTable<'a> {
)
}

/// Works almost same as [`Self::normalize_associated_types_in`], but this also resolves shallow
/// the inference variables
pub(crate) fn eagerly_normalize_and_resolve_shallow_in<T>(&mut self, ty: T) -> T
where
T: HasInterner<Interner = Interner> + TypeFoldable<Interner>,
{
fn eagerly_resolve_ty<const N: usize>(
table: &mut InferenceTable<'_>,
ty: Ty,
mut tys: SmallVec<[Ty; N]>,
) -> Ty {
if tys.contains(&ty) {
return ty;
}
tys.push(ty.clone());

match ty.kind(Interner) {
TyKind::Alias(AliasTy::Projection(proj_ty)) => {
let ty = table.normalize_projection_ty(proj_ty.clone());
eagerly_resolve_ty(table, ty, tys)
}
TyKind::InferenceVar(..) => {
let ty = table.resolve_ty_shallow(&ty);
eagerly_resolve_ty(table, ty, tys)
}
_ => ty,
}
}

fold_tys_and_consts(
ty,
|e, _| match e {
Either::Left(ty) => {
Either::Left(eagerly_resolve_ty::<8>(self, ty, SmallVec::new()))
}
Either::Right(c) => Either::Right(match &c.data(Interner).value {
chalk_ir::ConstValue::Concrete(cc) => match &cc.interned {
crate::ConstScalar::UnevaluatedConst(c_id, subst) => {
// FIXME: same as `normalize_associated_types_in`
if subst.len(Interner) == 0 {
if let Ok(eval) = self.db.const_eval(*c_id, subst.clone(), None) {
eval
} else {
unknown_const(c.data(Interner).ty.clone())
}
} else {
unknown_const(c.data(Interner).ty.clone())
}
}
_ => c,
},
_ => c,
}),
},
DebruijnIndex::INNERMOST,
)
}

pub(crate) fn normalize_projection_ty(&mut self, proj_ty: ProjectionTy) -> Ty {
let var = self.new_type_var();
let alias_eq = AliasEq { alias: AliasTy::Projection(proj_ty), ty: var.clone() };
Expand Down Expand Up @@ -918,7 +976,26 @@ impl<'a> InferenceTable<'a> {

/// Check if given type is `Sized` or not
pub(crate) fn is_sized(&mut self, ty: &Ty) -> bool {
fn short_circuit_trivial_tys(ty: &Ty) -> Option<bool> {
match ty.kind(Interner) {
TyKind::Scalar(..)
| TyKind::Ref(..)
| TyKind::Raw(..)
| TyKind::Never
| TyKind::FnDef(..)
| TyKind::Array(..)
| TyKind::Function(..) => Some(true),
TyKind::Slice(..) | TyKind::Str | TyKind::Dyn(..) => Some(false),
_ => None,
}
}

let mut ty = ty.clone();
ty = self.eagerly_normalize_and_resolve_shallow_in(ty);
if let Some(sized) = short_circuit_trivial_tys(&ty) {
return sized;
}

{
let mut structs = SmallVec::<[_; 8]>::new();
// Must use a loop here and not recursion because otherwise users will conduct completely
Expand All @@ -937,26 +1014,16 @@ impl<'a> InferenceTable<'a> {
// Structs can have DST as its last field and such cases are not handled
// as unsized by the chalk, so we do this manually.
ty = last_field_ty;
ty = self.eagerly_normalize_and_resolve_shallow_in(ty);
if let Some(sized) = short_circuit_trivial_tys(&ty) {
return sized;
}
} else {
break;
};
}
}

// Early return for some obvious types
if matches!(
ty.kind(Interner),
TyKind::Scalar(..)
| TyKind::Ref(..)
| TyKind::Raw(..)
| TyKind::Never
| TyKind::FnDef(..)
| TyKind::Array(..)
| TyKind::Function(_)
) {
return true;
}

let Some(sized) = self
.db
.lang_item(self.trait_env.krate, LangItem::Sized)
Expand Down
46 changes: 45 additions & 1 deletion crates/ide-diagnostics/src/handlers/invalid_cast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,8 +440,9 @@ fn main() {
q as *const [i32];
//^^^^^^^^^^^^^^^^^ error: cannot cast thin pointer `*const i32` to fat pointer `*const [i32]`

// FIXME: This should emit diagnostics but disabled to prevent many false positives
let t: *mut (dyn Trait + 'static) = 0 as *mut _;
//^^^^^^^^^^^ error: cannot cast `usize` to a fat pointer `*mut _`

Copy link
Member Author

@ShoyuVanilla ShoyuVanilla Mar 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have disabled diagnostics on casts into *const dyn Trait here

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

Lines 125 to 130 in 37acea8

// Chalk doesn't support trait upcasting and fails to solve some obvious goals
// when the trait environment contains some recursive traits (See issue #18047)
// We skip cast checks for such cases for now, until the next-gen solver.
if contains_dyn_trait(&self.cast_ty) {
return Ok(());
}

but this somehow survived it by hiding behinde inference var 😅

let mut fail: *const str = 0 as *const str;
//^^^^^^^^^^^^^^^ error: cannot cast `usize` to a fat pointer `*const str`
let mut fail2: *const str = 0isize as *const str;
Expand Down Expand Up @@ -1161,6 +1162,49 @@ struct ZerocopyKnownLayoutMaybeUninit(<<Flexible as Field>::Type as KnownLayout>
fn test(ptr: *mut [u8]) -> *mut ZerocopyKnownLayoutMaybeUninit {
ptr as *mut _
}
"#,
);
}

#[test]
fn regression_19431() {
check_diagnostics(
r#"
//- minicore: coerce_unsized
struct Dst([u8]);

struct Struct {
body: Dst,
}

trait Field {
type Type: ?Sized;
}

impl Field for Struct {
type Type = Dst;
}

trait KnownLayout {
type MaybeUninit: ?Sized;
type PointerMetadata;
}

impl<T> KnownLayout for [T] {
type MaybeUninit = [T];
type PointerMetadata = usize;
}

impl KnownLayout for Dst {
type MaybeUninit = Dst;
type PointerMetadata = <[u8] as KnownLayout>::PointerMetadata;
}

struct ZerocopyKnownLayoutMaybeUninit(<<Struct as Field>::Type as KnownLayout>::MaybeUninit);

fn test(ptr: *mut ZerocopyKnownLayoutMaybeUninit) -> *mut <<Struct as Field>::Type as KnownLayout>::MaybeUninit {
ptr as *mut _
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In zerocopy's proc macro expansion, there are so many nested projection types and inference vars and because of that the last field extracting stops along the way in the following function:

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

Lines 919 to 944 in 37acea8

/// Check if given type is `Sized` or not
pub(crate) fn is_sized(&mut self, ty: &Ty) -> bool {
let mut ty = ty.clone();
{
let mut structs = SmallVec::<[_; 8]>::new();
// Must use a loop here and not recursion because otherwise users will conduct completely
// artificial examples of structs that have themselves as the tail field and complain r-a crashes.
while let Some((AdtId::StructId(id), subst)) = ty.as_adt() {
let struct_data = self.db.variant_data(id.into());
if let Some((last_field, _)) = struct_data.fields().iter().next_back() {
let last_field_ty = self.db.field_types(id.into())[last_field]
.clone()
.substitute(Interner, subst);
if structs.contains(&ty) {
// A struct recursively contains itself as a tail field somewhere.
return true; // Don't overload the users with too many errors.
}
structs.push(ty);
// Structs can have DST as its last field and such cases are not handled
// as unsized by the chalk, so we do this manually.
ty = last_field_ty;
} else {
break;
};
}
}

Ideally, this should anyway return !Sized as the function body before trait solving is just a performance-related logic, but the chalk returns wrong unique solution to a goal struct Dst([u8]): Sized 🤔

So, I added a function eagerly resolves such nested inference vars and projection types.

}
"#,
);
}
Expand Down
Loading