From 783da4357becc7d5eea62fe7b6c3a1bd09109bb9 Mon Sep 17 00:00:00 2001 From: Abdullah Azmi Date: Mon, 19 Oct 2026 03:50:25 +0000 Subject: [PATCH] fix(hy-check): preserve generic struct type arguments across shape and loan checks `named_shape_depth` previously stripped generic type arguments via `base_named_type_str` (`box[[]int]` -> `TypeShape::Struct(Some("box"))`), and `shape_may_share_at`, `shape_carries_handle_at`, `field_shape`, `call_shape`, `ty_of_expr`, `no_zero_value_reason`, `place_target_ty`, `check_composite_nils`, and `shape_holds_*` looked up raw un-substituted field types in `self.structs.get("box")` where `T` resolved to `TypeShape::Unknown`. Preserve the instantiated generic struct name in `TypeShape::Struct(Some(name))` and substitute declared struct/function/method type parameters through `struct_fields_for_sname` and `fn_subst_for_state`, while normalizing receiver base names when looking up method declarations. Fixes #772 --- compiler/crates/hy-check/src/emit_guard.rs | 6 +- compiler/crates/hy-check/src/freeze.rs | 19 +- compiler/crates/hy-check/src/loans.rs | 94 +++++--- compiler/crates/hy-check/src/sendability.rs | 150 +++++++++--- compiler/crates/hy-check/src/shape.rs | 225 +++++++++++------- .../generic_struct_soundness_issue_772.rs | 200 ++++++++++++++++ 6 files changed, 541 insertions(+), 153 deletions(-) create mode 100644 compiler/crates/hy-check/tests/generic_struct_soundness_issue_772.rs diff --git a/compiler/crates/hy-check/src/emit_guard.rs b/compiler/crates/hy-check/src/emit_guard.rs index f47e6d7a7..13b7ea2fb 100644 --- a/compiler/crates/hy-check/src/emit_guard.rs +++ b/compiler/crates/hy-check/src/emit_guard.rs @@ -459,7 +459,7 @@ impl<'a> Checker<'a> { { sname = target.clone(); } - if let Some(struct_fields) = self.structs.get(&sname).cloned() { + if let Some(struct_fields) = self.struct_fields_for_sname(&sname) { for f in fields { if let Some((_, f_ty)) = struct_fields.iter().find(|(fname, _)| fname == &f.name) @@ -798,8 +798,8 @@ impl<'a> Checker<'a> { return None; // value cycle — Go rejects the type itself elsewhere } seen.push(name.clone()); - let fields = self.structs.get(name)?; - for (_, ft) in fields { + let fields = self.struct_fields_for_sname(name)?; + for (_, ft) in &fields { let fs = self.shape_of_type(ft); if let Some(n) = self.incomparable_rec(&fs, seen) { return Some(n); diff --git a/compiler/crates/hy-check/src/freeze.rs b/compiler/crates/hy-check/src/freeze.rs index 4d8ce9ebc..57f918e9d 100644 --- a/compiler/crates/hy-check/src/freeze.rs +++ b/compiler/crates/hy-check/src/freeze.rs @@ -1074,16 +1074,19 @@ impl<'a> Checker<'a> { self.shape_is_proven_pure_value(inner, depth + 1) } crate::model::TypeShape::Struct(Some(sname)) => { - if self.file.decls.iter().any(|d| { - matches!( - d, - hy_syntax::Decl::Struct(s) - if s.name == *sname && !s.type_params.is_empty() - ) - }) { + let base = Self::base_named_type_str(sname); + if !sname.contains('[') + && self.file.decls.iter().any(|d| { + matches!( + d, + hy_syntax::Decl::Struct(s) + if s.name == base && !s.type_params.is_empty() + ) + }) + { return false; } - let Some(fields) = self.structs.get(sname.as_str()) else { + let Some(fields) = self.struct_fields_for_sname(sname) else { return false; }; fields.iter().all(|(_, fty)| { diff --git a/compiler/crates/hy-check/src/loans.rs b/compiler/crates/hy-check/src/loans.rs index cc845c11b..e95761f59 100644 --- a/compiler/crates/hy-check/src/loans.rs +++ b/compiler/crates/hy-check/src/loans.rs @@ -594,7 +594,11 @@ impl<'a> Checker<'a> { if !live.iter().any(|p| a3_paths_conflict(st, p, &arg_path)) { continue; } - let Some(decl) = self.fns.get(name) else { + let Some(decl) = self + .fns + .get(name) + .or_else(|| self.fns.get(Self::base_named_type_str(name))) + else { continue; }; match decl.params.get(i) { @@ -672,18 +676,19 @@ impl<'a> Checker<'a> { return; } let recv_sname = match recv { - TypeShape::Struct(Some(s)) => Some(s.clone()), + TypeShape::Struct(Some(s)) => Some(Self::base_named_type_str(s).to_string()), TypeShape::Pointer(inner) => match &**inner { - TypeShape::Struct(Some(s)) => Some(s.clone()), + TypeShape::Struct(Some(s)) => Some(Self::base_named_type_str(s).to_string()), _ => None, }, _ => None, }; + let method_head = Self::base_named_type_str(method); let verb = format!("mutation via `{}`", cr.callee.render()); let callee_path = walk::expr_path(cr.callee).unwrap_or_else(|| root.to_string()); if let Some(sname) = recv_sname { let found_decl = self.file.decls.iter().find_map(|d| match d { - hy_syntax::Decl::Fn(f) if f.name == method => { + hy_syntax::Decl::Fn(f) if f.name == method_head => { let r = f.receiver.as_ref()?; let b = Self::base_named_type(&r.ty)?; (b == sname).then_some(f) @@ -985,7 +990,11 @@ impl<'a> Checker<'a> { if A3_PURE_FNS.contains(&name.as_str()) { return false; } - if let Some(decl) = self.fns.get(name) { + if let Some(decl) = self + .fns + .get(name) + .or_else(|| self.fns.get(Self::base_named_type_str(name))) + { for (i, a) in cr.args.iter().enumerate() { if !arg_root_is_param(a) { continue; @@ -1099,7 +1108,11 @@ impl<'a> Checker<'a> { } let mut mutated_roots = HashSet::new(); if let Expr::Ident { name: fn_name, .. } = unwrap_parens(callee) { - if let Some(decl) = self.fns.get(fn_name) { + if let Some(decl) = self + .fns + .get(fn_name) + .or_else(|| self.fns.get(Self::base_named_type_str(fn_name))) + { for (i, a) in args.iter().enumerate() { let mutates = match decl.params.get(i) { Some(p) => self.fn_mutates_param(decl, &p.name, 0), @@ -1315,7 +1328,11 @@ impl<'a> Checker<'a> { if A3_PURE_FNS.contains(&name.as_str()) { continue; } - if let Some(decl) = self.fns.get(name) { + if let Some(decl) = self + .fns + .get(name) + .or_else(|| self.fns.get(Self::base_named_type_str(name))) + { for (i, a) in cr.args.iter().enumerate() { let mut roots = Vec::new(); collect_ref_roots(&a.value, &mut roots); @@ -2883,7 +2900,10 @@ impl<'a> Checker<'a> { self.append_outer_binding_slots(st, &mut bound, true, &mut slots); return Some((slots, std::borrow::Cow::Owned(body.clone()))); } - let decl = *self.fns.get(name.as_str())?; + let decl = *self + .fns + .get(name.as_str()) + .or_else(|| self.fns.get(Self::base_named_type_str(name)))?; let body = decl.body.as_ref()?; let mut slots = Vec::new(); let mut bound: HashSet = @@ -6485,17 +6505,18 @@ impl<'a> Checker<'a> { } = callee { let recv_base = match self.shape_of_expr(st, base) { - TypeShape::Struct(Some(s)) => Some(s), + TypeShape::Struct(Some(s)) => Some(Self::base_named_type_str(&s).to_string()), TypeShape::Pointer(inner) => match *inner { - TypeShape::Struct(Some(s)) => Some(s), + TypeShape::Struct(Some(s)) => Some(Self::base_named_type_str(&s).to_string()), _ => None, }, _ => None, }; + let method_head = Self::base_named_type_str(method_name).to_string(); if let Some(rname) = recv_base { if let Some(decl) = self .method_decls - .get(&(rname, method_name.clone())) + .get(&(rname, method_head)) .copied() { if let Some(r) = &decl.receiver { @@ -6560,7 +6581,11 @@ impl<'a> Checker<'a> { Expr::Ident { name, .. } => { if declared.contains(name) { (false, true) - } else if let Some(decl) = self.fns.get(name) { + } else if let Some(decl) = self + .fns + .get(name) + .or_else(|| self.fns.get(Self::base_named_type_str(name))) + { match decl.params.get(i) { Some(p) => { let is_ptr = matches!(p.ty, hy_syntax::TypeExpr::Pointer { .. }) @@ -6611,15 +6636,20 @@ impl<'a> Checker<'a> { (false, true) } else { let recv_base = match self.shape_of_expr(st, base) { - TypeShape::Struct(Some(s)) => Some(s), + TypeShape::Struct(Some(s)) => { + Some(Self::base_named_type_str(&s).to_string()) + } TypeShape::Pointer(inner) => match *inner { - TypeShape::Struct(Some(s)) => Some(s), + TypeShape::Struct(Some(s)) => { + Some(Self::base_named_type_str(&s).to_string()) + } _ => None, }, _ => None, }; + let method_head = Self::base_named_type_str(method_name).to_string(); if let Some(decl) = recv_base.and_then(|rb| { - self.method_decls.get(&(rb, method_name.clone())).copied() + self.method_decls.get(&(rb, method_head)).copied() }) { match decl.params.get(i) { Some(p) => { @@ -6969,7 +6999,11 @@ impl<'a> Checker<'a> { } return; } - if let Some(decl) = self.fns.get(name) { + if let Some(decl) = self + .fns + .get(name) + .or_else(|| self.fns.get(Self::base_named_type_str(name))) + { let Some(body) = &decl.body else { return; }; @@ -6997,8 +7031,9 @@ impl<'a> Checker<'a> { name: method_name, .. } => { + let method_head = Self::base_named_type_str(method_name); for ((_, m), decl) in &self.method_decls { - if m != method_name { + if m != method_head { continue; } let Some(body) = &decl.body else { @@ -7182,7 +7217,11 @@ impl<'a> Checker<'a> { if A3_PURE_FNS.contains(&name.as_str()) { return false; } - if let Some(decl) = self.fns.get(name) { + if let Some(decl) = self + .fns + .get(name) + .or_else(|| self.fns.get(Self::base_named_type_str(name))) + { for (i, a) in cr.args.iter().enumerate() { if !arg_reaches_alias(a) { continue; @@ -7210,10 +7249,11 @@ impl<'a> Checker<'a> { return false; } } + let method_head = Self::base_named_type_str(method_name); let matching: Vec<&FnDecl> = self .method_decls .iter() - .filter(|((_, m), _)| m == method_name) + .filter(|((_, m), _)| m == method_head) .map(|(_, d)| *d) .collect(); if Self::expr_reaches_const_aliases(base, aliases) @@ -7327,37 +7367,37 @@ impl<'a> Checker<'a> { } PlaceKind::Ident(name) => { let b = self.lookup_binding(st, name)?; - let mut cur = b.ty.as_ref()?; + let mut cur = b.ty.clone()?; steps.reverse(); for step in steps { while let hy_syntax::TypeExpr::Pointer { to, .. } = cur { - cur = to.as_ref(); + cur = *to; } match step { Step::Index => match cur { hy_syntax::TypeExpr::Array { elem, .. } | hy_syntax::TypeExpr::Slice { elem, .. } => { - cur = elem.as_ref(); + cur = *elem; } hy_syntax::TypeExpr::Map { value, .. } => { - cur = value.as_ref(); + cur = *value; } _ => return None, }, - Step::Field(field_name) => match cur { + Step::Field(field_name) => match &cur { hy_syntax::TypeExpr::Named { name: struct_name, .. } => { - let fields = self.structs.get(struct_name)?; + let fields = self.struct_fields_for_sname(struct_name)?; let (_, field_ty) = - fields.iter().find(|(f, _)| f == field_name)?; + fields.into_iter().find(|(f, _)| f == field_name)?; cur = field_ty; } _ => return None, }, } } - return Some(cur.clone()); + return Some(cur); } } } diff --git a/compiler/crates/hy-check/src/sendability.rs b/compiler/crates/hy-check/src/sendability.rs index f658af916..cc024a0fb 100644 --- a/compiler/crates/hy-check/src/sendability.rs +++ b/compiler/crates/hy-check/src/sendability.rs @@ -168,7 +168,7 @@ impl<'a> Checker<'a> { return false; } seen.push(sname.clone()); - let Some(fields) = self.structs.get(sname) else { + let Some(fields) = self.struct_fields_for_sname(sname) else { return false; }; fields @@ -200,7 +200,7 @@ impl<'a> Checker<'a> { return false; } seen.push(sname.clone()); - let Some(fields) = self.structs.get(sname) else { + let Some(fields) = self.struct_fields_for_sname(sname) else { return false; }; fields @@ -227,7 +227,7 @@ impl<'a> Checker<'a> { return false; } seen.push(sname.clone()); - let Some(fields) = self.structs.get(sname) else { + let Some(fields) = self.struct_fields_for_sname(sname) else { return false; }; fields @@ -638,11 +638,26 @@ impl<'a> Checker<'a> { if self.lookup_binding(st, name).is_some() { return None; // shadowed by a local binding — not a static fn call } - let f = self.fns.get(name)?; + let head = Self::base_named_type_str(name); + let f = self.fns.get(name.as_str()).or_else(|| self.fns.get(head))?; if f.params.len() != argc { return None; } - Some(f.params.iter().map(|p| self.shape_of_type(&p.ty)).collect()) + let mut subst = std::collections::HashMap::new(); + if let Some((_, explicit_args)) = Self::parse_generic_type_args(name) { + for (tp, arg) in f.type_params.iter().zip(explicit_args.iter()) { + subst.insert(tp.name.clone(), arg.clone()); + } + } + Some( + f.params + .iter() + .map(|p| { + let pty = Self::substitute_type_params(&p.ty, &subst); + self.shape_of_type(&pty) + }) + .collect(), + ) } Expr::FnLit { params, .. } => { if params.len() != argc { @@ -656,15 +671,18 @@ impl<'a> Checker<'a> { .. } => { let recv_base = match self.shape_of_expr(st, base) { - TypeShape::Struct(Some(s)) => Some(s), + TypeShape::Struct(Some(s)) => Some(Self::base_named_type_str(&s).to_string()), TypeShape::Pointer(inner) => match *inner { - TypeShape::Struct(Some(s)) => Some(s), + TypeShape::Struct(Some(s)) => { + Some(Self::base_named_type_str(&s).to_string()) + } _ => None, }, _ => None, }; + let head = Self::base_named_type_str(method_name); let f = recv_base.and_then(|rb| { - self.method_decls.get(&(rb, method_name.clone())).copied() + self.method_decls.get(&(rb, head.to_string())).copied() })?; if f.params.len() != argc { return None; @@ -687,7 +705,11 @@ impl<'a> Checker<'a> { (Struct(None), _) | (_, Struct(None)) => true, (ForeignOpaque(_), _) | (_, ForeignOpaque(_)) => true, (Primitive, Primitive) | (Str, Str) | (Atomic, Atomic) => true, - (Struct(Some(a)), Struct(Some(b))) => a == b, + (Struct(Some(a)), Struct(Some(b))) => { + a == b + || (Self::base_named_type_str(a) == Self::base_named_type_str(b) + && (!a.contains('[') || !b.contains('['))) + } (Map(_, _), Map(_, _)) => true, (Slice(_), Slice(_)) | (Array(_), Array(_)) => true, // Go converts between slice and array of the same element class @@ -1011,7 +1033,8 @@ impl<'a> Checker<'a> { _ => None, } }) { - if let Some(def) = self.structs.get(&sname).cloned() { + if let Some(def) = self.struct_fields_for_sname(&sname) { + let base_sname = Self::base_named_type_str(&sname); for f in fields { if let Some((_, fty)) = def.iter().find(|(n, _)| n == &f.name) { if Checker::is_nil_literal(&f.value) { @@ -1021,7 +1044,7 @@ impl<'a> Checker<'a> { &fshape, fty.render(), f.value.span(), - &format!("the field `{}.{}`", sname, f.name), + &format!("the field `{}.{}`", base_sname, f.name), ); } continue; @@ -2184,7 +2207,7 @@ impl<'a> Checker<'a> { } } - fn parse_generic_type_args(name: &str) -> Option<(&str, Vec)> { + pub(crate) fn parse_generic_type_args(name: &str) -> Option<(&str, Vec)> { let open = name.find('[')?; if !name.ends_with(']') || open == 0 { return None; @@ -2225,7 +2248,7 @@ impl<'a> Checker<'a> { Some((base, parsed)) } - fn substitute_type_params( + pub(crate) fn substitute_type_params( ty: &hy_syntax::TypeExpr, subst: &std::collections::HashMap, ) -> hy_syntax::TypeExpr { @@ -2300,7 +2323,7 @@ impl<'a> Checker<'a> { } } - fn peel_pointer_and_alias(&self, ty: &hy_syntax::TypeExpr) -> hy_syntax::TypeExpr { + pub(crate) fn peel_pointer_and_alias(&self, ty: &hy_syntax::TypeExpr) -> hy_syntax::TypeExpr { let mut cur = self.resolve_type_alias(ty); while let hy_syntax::TypeExpr::Pointer { to, .. } = cur { cur = self.resolve_type_alias(&to); @@ -2308,23 +2331,28 @@ impl<'a> Checker<'a> { cur } - fn struct_fields_for_type( + pub(crate) fn struct_fields_for_sname( &self, - ty: &hy_syntax::TypeExpr, + raw_name: &str, ) -> Option> { - let peeled = self.peel_pointer_and_alias(ty); - let hy_syntax::TypeExpr::Named { name, .. } = &peeled else { - return None; - }; - let sname = Self::base_named_type_str(name); + let mut cur = raw_name.trim().to_string(); + for _ in 0..8 { + if let Some(hy_syntax::TypeExpr::Named { name, .. }) = + self.type_aliases.get(cur.as_str()) + { + if name != &cur { + cur = name.clone(); + continue; + } + } + break; + } + let sname = Self::base_named_type_str(&cur); let fields = self.structs.get(sname)?.clone(); - if let Some((_, type_args)) = Self::parse_generic_type_args(name) { - if let Some(sdecl) = self.file.decls.iter().find_map(|d| match d { - hy_syntax::Decl::Struct(s) if s.name == sname => Some(s), - _ => None, - }) { + if let Some((_, type_args)) = Self::parse_generic_type_args(&cur) { + if let Some(tparams) = self.struct_type_params.get(sname) { let mut subst = std::collections::HashMap::new(); - for (tp, arg) in sdecl.type_params.iter().zip(type_args.iter()) { + for (tp, arg) in tparams.iter().zip(type_args.iter()) { subst.insert(tp.name.clone(), arg.clone()); } if !subst.is_empty() { @@ -2340,6 +2368,74 @@ impl<'a> Checker<'a> { Some(fields) } + pub(crate) fn struct_fields_for_type( + &self, + ty: &hy_syntax::TypeExpr, + ) -> Option> { + let peeled = self.peel_pointer_and_alias(ty); + let hy_syntax::TypeExpr::Named { name, .. } = &peeled else { + return None; + }; + self.struct_fields_for_sname(name) + } + + pub(crate) fn fn_subst_for_state( + &self, + st: &FnState, + f: &hy_syntax::FnDecl, + callee_name: &str, + receiver_ty: Option<&hy_syntax::TypeExpr>, + args: &[hy_syntax::CallArg], + ) -> std::collections::HashMap { + let mut subst = std::collections::HashMap::new(); + if let (Some(rdecl), Some(ract)) = (&f.receiver, receiver_ty) { + let rdecl_peeled = self.peel_pointer_and_alias(&rdecl.ty); + let ract_peeled = self.peel_pointer_and_alias(ract); + if let ( + hy_syntax::TypeExpr::Named { name: dn, .. }, + hy_syntax::TypeExpr::Named { name: an, .. }, + ) = (&rdecl_peeled, &ract_peeled) + { + if let (Some((db, dargs)), Some((ab, aargs))) = ( + Self::parse_generic_type_args(dn), + Self::parse_generic_type_args(an), + ) { + if db == ab { + for (darg, aarg) in dargs.iter().zip(aargs.iter()) { + if let hy_syntax::TypeExpr::Named { name: tp_name, .. } = darg { + subst.insert(tp_name.clone(), aarg.clone()); + } + } + } + } + } + } + if let Some((_, explicit_args)) = Self::parse_generic_type_args(callee_name) { + for (tp, arg) in f.type_params.iter().zip(explicit_args.iter()) { + subst.insert(tp.name.clone(), arg.clone()); + } + } else if !f.type_params.is_empty() { + let tp_names: std::collections::HashSet = + f.type_params.iter().map(|tp| tp.name.clone()).collect(); + for (i, arg) in args.iter().enumerate() { + let param = if let Some(aname) = &arg.name { + f.params + .iter() + .find(|p| &p.name == aname) + .or_else(|| f.params.get(i)) + } else { + f.params.get(i) + }; + if let Some(p) = param { + if let Some(arg_ty) = self.ty_of_expr(st, &arg.value) { + self.infer_type_param_bindings(&p.ty, &arg_ty, &tp_names, &mut subst); + } + } + } + } + subst + } + fn infer_type_param_bindings( &self, param_ty: &hy_syntax::TypeExpr, diff --git a/compiler/crates/hy-check/src/shape.rs b/compiler/crates/hy-check/src/shape.rs index e24f61369..b338f211a 100644 --- a/compiler/crates/hy-check/src/shape.rs +++ b/compiler/crates/hy-check/src/shape.rs @@ -79,11 +79,11 @@ impl<'a> Checker<'a> { // see `c.order` as the declared `[]k`, not an opaque Unknown). let base = Self::base_named_type_str(name); // User structs in package scope take precedence over predeclared types. - if self.structs.contains_key(base) { - return TypeShape::Struct(Some(base.to_string())); + if self.structs.contains_key(base) || self.struct_type_params.contains_key(base) { + return TypeShape::Struct(Some(name.to_string())); } // User type aliases take precedence over predeclared types. - if let Some(target) = self.type_aliases.get(base) { + if let Some(target) = self.type_aliases.get(name).or_else(|| self.type_aliases.get(base)) { return self.shape_of_type_depth(target, depth + 1); } match name { @@ -130,8 +130,7 @@ impl<'a> Checker<'a> { match shape { TypeShape::Array(elem) => self.shape_may_share_at(elem, depth + 1), TypeShape::Struct(Some(name)) => self - .structs - .get(name) + .struct_fields_for_sname(name) .map(|fields| { fields .iter() @@ -169,8 +168,7 @@ impl<'a> Checker<'a> { || self.shape_carries_handle_at(v, depth + 1) } TypeShape::Struct(Some(name)) => self - .structs - .get(name) + .struct_fields_for_sname(name) .map(|fields| { fields.iter().any(|(_, ty)| { self.shape_carries_handle_at(&self.shape_of_type(ty), depth + 1) @@ -184,8 +182,7 @@ impl<'a> Checker<'a> { /// Does this struct decl guard something with a `sync.*` lock? (Used for /// the contested-04 note: "Tier 0 does not credit locks at spawn sites".) pub(crate) fn struct_has_mutex_field(&self, sname: &str) -> bool { - self.structs - .get(sname) + self.struct_fields_for_sname(sname) .map(|fields| { fields.iter().any(|(_, ty)| { matches!( @@ -301,10 +298,9 @@ impl<'a> Checker<'a> { pub(crate) fn field_shape(&self, base: &TypeShape, field: &str) -> TypeShape { match base { TypeShape::Struct(Some(sname)) => self - .structs - .get(sname) - .and_then(|fs| fs.iter().find(|(n, _)| n == field)) - .map(|(_, t)| self.shape_of_type(t)) + .struct_fields_for_sname(sname) + .and_then(|fs| fs.into_iter().find(|(n, _)| n == field)) + .map(|(_, t)| self.shape_of_type(&t)) .unwrap_or(TypeShape::Unknown), TypeShape::Pointer(inner) => self.field_shape(inner, field), _ => TypeShape::Unknown, @@ -387,8 +383,12 @@ impl<'a> Checker<'a> { self.fns .get(name.as_str()) .or_else(|| self.fns.get(head)) - .and_then(|f| f.results.first()) - .map(|r| self.shape_of_type(&r.ty)) + .and_then(|f| { + let r = f.results.first()?; + let subst = self.fn_subst_for_state(st, f, name, None, args); + let rty = Self::substitute_type_params(&r.ty, &subst); + Some(self.shape_of_type(&rty)) + }) .unwrap_or(TypeShape::Unknown) } } @@ -408,7 +408,11 @@ impl<'a> Checker<'a> { return f .results .first() - .map(|r| self.shape_of_type(&r.ty)) + .map(|r| { + let subst = self.fn_subst_for_state(st, f, name, None, args); + let rty = Self::substitute_type_params(&r.ty, &subst); + self.shape_of_type(&rty) + }) .unwrap_or(TypeShape::Unknown); } } @@ -420,55 +424,38 @@ impl<'a> Checker<'a> { return (**elem).clone(); } } - if let Some(recv_base) = self - .ty_of_expr(st, base) - .map(|t| self.resolve_type_alias(&t)) - .and_then(|t| Self::base_named_type(&t)) - .or_else(|| match &recv { - TypeShape::Struct(Some(s)) => { - Some(Self::base_named_type_str(s).to_string()) - } - TypeShape::Pointer(inner) => match &**inner { - TypeShape::Struct(Some(s)) => { - Some(Self::base_named_type_str(s).to_string()) - } - _ => None, - }, + let recv_ty = self.ty_of_expr(st, base).or_else(|| match &recv { + TypeShape::Struct(Some(s)) => Some(TypeExpr::Named { + name: s.clone(), + span: base.span(), + }), + TypeShape::Pointer(inner) => match &**inner { + TypeShape::Struct(Some(s)) => Some(TypeExpr::Named { + name: s.clone(), + span: base.span(), + }), _ => None, - }) + }, + _ => None, + }); + if let Some(recv_base) = recv_ty + .as_ref() + .map(|t| self.resolve_type_alias(t)) + .and_then(|t| Self::base_named_type(&t)) { if let Some(f) = self.method_decls.get(&(recv_base, head.to_string())) { if let Some(r) = f.results.first() { - return self.shape_of_type(&r.ty); + let subst = + self.fn_subst_for_state(st, f, name, recv_ty.as_ref(), args); + let rty = Self::substitute_type_params(&r.ty, &subst); + return self.shape_of_type(&rty); } } } - let recv = self.shape_of_expr(st, base); if let Some(t) = receiver_type_name(&recv) { return foreign::method_result(&t, name); } - let head = Self::base_named_type_str(name); - let recv_base = self - .ty_of_expr(st, base) - .map(|t| self.resolve_type_alias(&t)) - .and_then(|t| Self::base_named_type(&t)) - .or_else(|| match &recv { - TypeShape::Struct(Some(s)) => { - Some(Self::base_named_type_str(s).to_string()) - } - TypeShape::Pointer(inner) => match &**inner { - TypeShape::Struct(Some(s)) => { - Some(Self::base_named_type_str(s).to_string()) - } - _ => None, - }, - _ => None, - }); - recv_base - .and_then(|rb| self.method_decls.get(&(rb, head.to_string()))) - .and_then(|f| f.results.first()) - .map(|r| self.shape_of_type(&r.ty)) - .unwrap_or(TypeShape::Unknown) + TypeShape::Unknown } _ => TypeShape::Unknown, } @@ -640,6 +627,12 @@ impl<'a> Checker<'a> { }); } } + let inner = Self::peel_pointer_and_alias(self, &resolved); + if let Some(fs) = self.struct_fields_for_type(&inner) { + if let Some((_, t)) = fs.into_iter().find(|(n, _)| n == name) { + return Some(t); + } + } let sname = Self::base_named_type(&resolved)?; self.structs .get(&sname) @@ -737,11 +730,14 @@ impl<'a> Checker<'a> { } } let head = Self::base_named_type_str(name); - self.fns - .get(name.as_str()) - .or_else(|| self.fns.get(head)) - .and_then(|f| f.results.first()) - .map(|r| r.ty.clone()) + let f = self.fns.get(name.as_str()).or_else(|| self.fns.get(head))?; + let ret = f.results.first()?.ty.clone(); + let subst = self.fn_subst_for_state(st, f, name, None, args); + Some(if subst.is_empty() { + ret + } else { + Self::substitute_type_params(&ret, &subst) + }) } Expr::Field { base, name, .. } => { let head = Self::base_named_type_str(name); @@ -749,13 +745,20 @@ impl<'a> Checker<'a> { if self.lookup_binding(st, root).is_none() { let full = format!("{}.{}", root, head); if let Some(f) = self.fns.get(full.as_str()) { - return f.results.first().map(|r| r.ty.clone()); + let ret = f.results.first()?.ty.clone(); + let subst = self.fn_subst_for_state(st, f, name, None, args); + return Some(if subst.is_empty() { + ret + } else { + Self::substitute_type_params(&ret, &subst) + }); } } } - let recv_base = self - .ty_of_expr(st, base) - .map(|t| self.resolve_type_alias(&t)) + let recv_ty = self.ty_of_expr(st, base); + let recv_base = recv_ty + .as_ref() + .map(|t| self.resolve_type_alias(t)) .and_then(|t| Self::base_named_type(&t)) .or_else(|| match self.shape_of_expr(st, base) { TypeShape::Struct(Some(s)) => { @@ -770,13 +773,16 @@ impl<'a> Checker<'a> { _ => None, }); if let Some(rb) = recv_base { - if let Some(ret) = self - .method_decls - .get(&(rb, head.to_string())) - .and_then(|f| f.results.first()) - .map(|r| r.ty.clone()) - { - return Some(ret); + if let Some(f) = self.method_decls.get(&(rb, head.to_string())) { + if let Some(r) = f.results.first() { + let subst = + self.fn_subst_for_state(st, f, name, recv_ty.as_ref(), args); + return Some(if subst.is_empty() { + r.ty.clone() + } else { + Self::substitute_type_params(&r.ty, &subst) + }); + } } } let callee_ty = self.ty_of_expr(st, callee)?; @@ -833,8 +839,8 @@ impl<'a> Checker<'a> { base )); } - if let Some(fields) = self.structs.get(base) { - for (fname, fty) in fields { + if let Some(fields) = self.struct_fields_for_sname(name) { + for (fname, fty) in &fields { if let Some(inner) = self.no_zero_value_reason(fty, depth + 1) { return Some(format!( "struct `{}` has no zero value because field `{}` ({})", @@ -1099,6 +1105,15 @@ impl<'a> Checker<'a> { } pub(crate) fn call_result_tys(&self, st: &FnState, callee: &Expr) -> Option> { + self.call_result_tys_with_args(st, callee, &[]) + } + + pub(crate) fn call_result_tys_with_args( + &self, + st: &FnState, + callee: &Expr, + args: &[hy_syntax::CallArg], + ) -> Option> { match crate::model::unwrap_parens(callee) { Expr::FnLit { results, .. } => Some(results.iter().map(|r| r.ty.clone()).collect()), Expr::Ident { name, .. } => { @@ -1111,10 +1126,20 @@ impl<'a> Checker<'a> { return None; } let head = Self::base_named_type_str(name); - self.fns - .get(name.as_str()) - .or_else(|| self.fns.get(head)) - .map(|f| f.results.iter().map(|r| r.ty.clone()).collect()) + let f = self.fns.get(name.as_str()).or_else(|| self.fns.get(head))?; + let subst = self.fn_subst_for_state(st, f, name, None, args); + Some( + f.results + .iter() + .map(|r| { + if subst.is_empty() { + r.ty.clone() + } else { + Self::substitute_type_params(&r.ty, &subst) + } + }) + .collect(), + ) } Expr::Field { base, name, .. } => { let head = Self::base_named_type_str(name); @@ -1122,14 +1147,27 @@ impl<'a> Checker<'a> { if self.lookup_binding(st, root).is_none() { let full = format!("{}.{}", root, head); if let Some(f) = self.fns.get(full.as_str()) { - return Some(f.results.iter().map(|r| r.ty.clone()).collect()); + let subst = self.fn_subst_for_state(st, f, name, None, args); + return Some( + f.results + .iter() + .map(|r| { + if subst.is_empty() { + r.ty.clone() + } else { + Self::substitute_type_params(&r.ty, &subst) + } + }) + .collect(), + ); } } } let recv = self.shape_of_expr(st, base); - let recv_base = self - .ty_of_expr(st, base) - .map(|t| self.resolve_type_alias(&t)) + let recv_ty = self.ty_of_expr(st, base); + let recv_base = recv_ty + .as_ref() + .map(|t| self.resolve_type_alias(t)) .and_then(|t| Self::base_named_type(&t)) .or_else(|| match &recv { TypeShape::Struct(Some(s)) => { @@ -1143,9 +1181,20 @@ impl<'a> Checker<'a> { }, _ => None, })?; - self.method_decls - .get(&(recv_base, head.to_string())) - .map(|f| f.results.iter().map(|r| r.ty.clone()).collect()) + let f = self.method_decls.get(&(recv_base, head.to_string()))?; + let subst = self.fn_subst_for_state(st, f, name, recv_ty.as_ref(), args); + Some( + f.results + .iter() + .map(|r| { + if subst.is_empty() { + r.ty.clone() + } else { + Self::substitute_type_params(&r.ty, &subst) + } + }) + .collect(), + ) } _ => None, } @@ -1180,8 +1229,8 @@ impl<'a> Checker<'a> { Expr::Try { operand, .. } => (crate::model::unwrap_parens(operand), true), other => (other, false), }; - if let Expr::Call { callee, .. } = call_expr { - if let Some(results) = self.call_result_tys(st, callee) { + if let Expr::Call { callee, args, .. } = call_expr { + if let Some(results) = self.call_result_tys_with_args(st, callee, args) { let avail = if is_try { results.len().saturating_sub(1) } else { @@ -1219,8 +1268,8 @@ impl<'a> Checker<'a> { Expr::Try { operand, .. } => (crate::model::unwrap_parens(operand), true), other => (other, false), }; - if let Expr::Call { callee, .. } = call_expr { - if let Some(results) = self.call_result_tys(st, callee) { + if let Expr::Call { callee, args, .. } = call_expr { + if let Some(results) = self.call_result_tys_with_args(st, callee, args) { let avail = if is_try { results.len().saturating_sub(1) } else { diff --git a/compiler/crates/hy-check/tests/generic_struct_soundness_issue_772.rs b/compiler/crates/hy-check/tests/generic_struct_soundness_issue_772.rs new file mode 100644 index 000000000..45ce9803d --- /dev/null +++ b/compiler/crates/hy-check/tests/generic_struct_soundness_issue_772.rs @@ -0,0 +1,200 @@ +use hy_check::check_file; +use hy_syntax::parse_file; + +fn codes(src: &str) -> Vec { + let file = parse_file("test.hy", src).expect("unexpected parse errors"); + check_file(&file) + .diagnostics + .into_iter() + .map(|d| d.code.to_string()) + .collect() +} + +#[test] +fn generic_struct_tier0_alias_freeze_and_non_owned_move_rejected() { + let src = r#"package main + +struct box[t] { + v t +} + +fn worker(b box[[]int], done chan<- bool) { + b.v[0] = 1 + done <- true +} + +fn leak_move(b box[[]int]) []int { + return move b.v +} + +fn main() { + done := make(chan bool, 1) + a := box[[]int]{v: []int{10, 20}} + b := a + task worker(move a, done) + b.v[0] = 99 + <-done + _ = leak_move(b) +} +"#; + let got = codes(src); + assert!( + got.iter().any(|c| c == "HYB-E3003"), + "expected HYB-E3003 for mutating alias `b` of generic struct holding slice after `move a` into task, got {got:?}" + ); + assert!( + got.iter().any(|c| c == "HYB-E3004"), + "expected HYB-E3004 for moving slice out of non-owned generic struct parameter, got {got:?}" + ); +} + +#[test] +fn generic_struct_const_deep_mutation_rejected() { + let src = r#"package main + +struct box[t] { + v t +} + +const cb box[[]int] = box[[]int]{v: []int{1, 2, 3}} + +fn main() { + local := cb + local.v[0] = 777 + alias := cb.v + alias[1] = 888 +} +"#; + let got = codes(src); + let e3006_count = got.iter().filter(|c| c.as_str() == "HYB-E3006").count(); + assert!( + e3006_count >= 2, + "expected HYB-E3006 on both local.v[0] and alias[1] writes into const box[[]int], got {got:?}" + ); +} + +#[test] +fn generic_struct_move_required_and_use_after_move_rejected() { + let src = r#"package main + +struct box[t] { + v t +} + +fn worker(b box[[]int], done chan<- bool) { + done <- true +} + +fn main() { + done := make(chan bool, 2) + ch := make(chan box[[]int], 1) + a := box[[]int]{v: []int{1, 2}} + task worker(a, done) + ch <- a + + b := box[[]int]{v: []int{3, 4}} + task worker(move b, done) + _ = b.v[0] + <-done +} +"#; + let got = codes(src); + assert!( + got.iter().any(|c| c == "HYB-E3001"), + "expected HYB-E3001 for spawning task with box[[]int] without `move`, got {got:?}" + ); + assert!( + got.iter().any(|c| c == "HYB-E3002"), + "expected HYB-E3002 for sending box[[]int] over channel without `move`, got {got:?}" + ); + assert!( + got.iter().any(|c| c == "HYB-E3003"), + "expected HYB-E3003 for reusing generic struct box[[]int] after move, got {got:?}" + ); +} + +#[test] +fn generic_struct_map_slot_loan_a3_rejected() { + let src = r#"package main + +struct svc { + count int +} + +struct box[t] { + v t +} + +fn main() { + b := box[map[string]svc]{v: map[string]svc{"a": svc{count: 1}}} + s := &b.v["a"] + b.v["b"] = svc{count: 2} + s.count = 99 +} +"#; + let got = codes(src); + assert!( + got.iter().any(|c| c == "HYB-E3102"), + "expected HYB-E3102 when inserting into map field of generic struct while slot loan `&b.v[\"a\"]` is live, got {got:?}" + ); +} + +#[test] +fn generic_struct_n1_nil_field_and_n3_no_zero_value_rejected() { + let src = r#"package main + +enum msg { + data(int) + empty +} + +struct box[t] { + v t +} + +fn main() { + b := box[[]int]{v: nil} + b.v = nil + var u box[msg] + _ = b + _ = u +} +"#; + let got = codes(src); + let e4001_count = got.iter().filter(|c| c.as_str() == "HYB-E4001").count(); + assert!( + e4001_count >= 2, + "expected HYB-E4001 on both struct literal nil field and field nil assignment on box[[]int], got {got:?}" + ); + assert!( + got.iter().any(|c| c == "HYB-E4002"), + "expected HYB-E4002 on uninitialized `var u box[msg]` where `msg` has no zero value, got {got:?}" + ); +} + +#[test] +fn generic_struct_pure_value_instantiation_allows_copy_and_zero_value() { + let src = r#"package main + +struct box[t] { + v t +} + +const cb box[int] = box[int]{v: 42} + +fn main() { + var z box[int] + a := box[int]{v: 10} + b := a + a.v = 20 + local := cb + local.v = 99 + _ = z.v + b.v + local.v +} +"#; + let got = codes(src); + assert!( + got.is_empty(), + "expected pure-value generic struct `box[int]` to allow copy, mutation of copy, and zero-value var, got {got:?}" + ); +}