Skip to content

fix(hy-check): preserve generic struct type args in TypeShape::Struct across all soundness passes (Fixes #775) #792

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

Open
huynhnhatquandongnai-source wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(hy-check): preserve generic struct type args in TypeShape::Struct across all soundness passes (Fixes #775) #792

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
5 changes: 3 additions & 2 deletions compiler/crates/hy-check/src/closures.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1141,8 +1141,9 @@ fn check_stmt_boundaries<'a>(ck: &mut Checker<'_>, st: &mut FnState, stmt: &'a S
if targets.len() == values.len() {
for (t, v) in targets.iter().zip(values.iter()) {
if matches!(t.kind, hy_syntax::PlaceKind::Field { .. }) {
let shape = ck.shape_of_expr(st, v);
let is_borrow = crate::model::as_addr_of(v).is_some()
|| matches!(ck.shape_of_expr(st, v), TypeShape::BorrowHandle(_));
|| ck.field_borrow_kind(&shape).is_some();
if is_borrow {
ck.emit_a6_field_borrow_error(t);
}
Expand All @@ -1152,7 +1153,7 @@ fn check_stmt_boundaries<'a>(ck: &mut Checker<'_>, st: &mut FnState, stmt: &'a S
let shapes = ck.multi_value_shapes(st, &values[0], targets.len());
for (t, shape) in targets.iter().zip(shapes.iter()) {
if matches!(t.kind, hy_syntax::PlaceKind::Field { .. })
&& matches!(shape, TypeShape::BorrowHandle(_))
&& ck.field_borrow_kind(shape).is_some()
{
ck.emit_a6_field_borrow_error(t);
}
Expand Down
29 changes: 19 additions & 10 deletions compiler/crates/hy-check/src/emit_guard.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,13 +410,8 @@ impl<'a> Checker<'a> {
self.check_type_expr_with_state(Some(st), t);
}
if let Some(TypeExpr::Named { name, .. }) = ty {
let mut sname = name.clone();
while let Some(TypeExpr::Named { name: target, .. }) =
self.type_aliases.get(&sname)
{
sname = target.clone();
}
if let Some(struct_fields) = self.structs.get(&sname).cloned() {
let sname = name.clone();
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)
Expand Down Expand Up @@ -755,8 +750,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);
Expand Down Expand Up @@ -957,7 +952,21 @@ impl<'a> Checker<'a> {
self.check_type_expr_with_state(st, r);
}
}
TypeExpr::Named { .. } => {}
TypeExpr::Named { name, span } => {
if let Some((base, type_args)) = Self::parse_generic_type_args(name) {
for arg in &type_args {
self.check_type_expr_with_state(st, arg);
}
if self.structs.contains_key(base) {
if let Some(fields) = self.struct_fields_for_sname(name) {
let sname = base.to_string();
for (fname, fty) in fields {
self.emit_struct_field_borrow_diag(&sname, &fname, &fty, *span);
}
}
}
}
}
}
}

Expand Down
5 changes: 3 additions & 2 deletions compiler/crates/hy-check/src/freeze.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1074,16 +1074,17 @@ impl<'a> Checker<'a> {
self.shape_is_proven_pure_value(inner, depth + 1)
}
crate::model::TypeShape::Struct(Some(sname)) => {
let base = Self::base_named_type_str(sname);
if self.file.decls.iter().any(|d| {
matches!(
d,
hy_syntax::Decl::Struct(s)
if s.name == *sname && !s.type_params.is_empty()
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)| {
Expand Down
198 changes: 122 additions & 76 deletions compiler/crates/hy-check/src/loans.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -672,9 +672,9 @@ 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,
Expand Down Expand Up @@ -1968,7 +1968,7 @@ impl<'a> Checker<'a> {
}
Expr::Field { base, .. } | Expr::Index { base, .. } => {
let sh = self.shape_of_expr(st, e);
if field_borrow_kind(&sh).is_some() || matches!(sh, TypeShape::Unknown) {
if self.field_borrow_kind(&sh).is_some() || matches!(sh, TypeShape::Unknown) {
self.collect_expr_addr_of_roots(st, base, out);
}
}
Expand Down Expand Up @@ -2009,12 +2009,12 @@ impl<'a> Checker<'a> {
if let Some(results) = self.call_result_tys(st, callee) {
return results.iter().any(|ty| {
let shape = self.shape_of_type(ty);
field_borrow_kind(&shape).is_some() || matches!(shape, TypeShape::Unknown)
self.field_borrow_kind(&shape).is_some() || matches!(shape, TypeShape::Unknown)
});
}
}
let shape = self.shape_of_expr(st, inner);
field_borrow_kind(&shape).is_some() || matches!(shape, TypeShape::Unknown)
self.field_borrow_kind(&shape).is_some() || matches!(shape, TypeShape::Unknown)
}

pub(crate) fn expr_carries_borrow(&self, st: &FnState, e: &Expr) -> bool {
Expand Down Expand Up @@ -3707,7 +3707,7 @@ impl<'a> Checker<'a> {
if name == "_" {
continue;
}
let borrow_slot = field_borrow_kind(&shape).is_some() || matches!(shape, TypeShape::Unknown);
let borrow_slot = self.field_borrow_kind(&shape).is_some() || matches!(shape, TypeShape::Unknown);
if (l.names.len() == l.values.len() || borrow_slot) && self.expr_carries_borrow(st, value) {
st.borrow_bindings.insert(name.clone());
} else {
Expand Down Expand Up @@ -3774,7 +3774,7 @@ impl<'a> Checker<'a> {
continue;
}
let borrow_slot =
field_borrow_kind(&shape).is_some() || matches!(shape, TypeShape::Unknown);
self.field_borrow_kind(&shape).is_some() || matches!(shape, TypeShape::Unknown);
let inner = unwrap_parens(v);
let is_borrow = as_addr_of(inner).is_some()
|| contains_addr_of_in_literal(inner)
Expand Down Expand Up @@ -5352,7 +5352,7 @@ impl<'a> Checker<'a> {
if matches!(t.kind, PlaceKind::Field { .. }) {
let shape = self.shape_of_expr(st, v);
let is_borrow = as_addr_of(v).is_some()
|| field_borrow_kind(&shape).is_some();
|| self.field_borrow_kind(&shape).is_some();
if is_borrow {
self.emit_a6_field_borrow_error(t);
}
Expand All @@ -5362,7 +5362,7 @@ impl<'a> Checker<'a> {
let shapes = self.multi_value_shapes(st, &values[0], targets.len());
for (t, shape) in targets.iter().zip(shapes.iter()) {
if matches!(t.kind, PlaceKind::Field { .. })
&& field_borrow_kind(shape).is_some()
&& self.field_borrow_kind(shape).is_some()
{
self.emit_a6_field_borrow_error(t);
}
Expand Down Expand Up @@ -6484,9 +6484,9 @@ 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,
Expand Down Expand Up @@ -6610,9 +6610,13 @@ 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,
Expand Down Expand Up @@ -7326,37 +7330,32 @@ 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();
}
_ => return None,
},
Step::Field(field_name) => match cur {
hy_syntax::TypeExpr::Named {
name: struct_name, ..
} => {
let fields = self.structs.get(struct_name)?;
let (_, field_ty) =
fields.iter().find(|(f, _)| f == field_name)?;
cur = field_ty;
cur = *value;
}
_ => return None,
},
Step::Field(field_name) => {
let fields = self.struct_fields_for_type(&cur)?;
let (_, field_ty) =
fields.into_iter().find(|(f, _)| f == field_name)?;
cur = field_ty;
}
}
}
return Some(cur.clone());
return Some(cur);
}
}
}
Expand Down Expand Up @@ -7776,46 +7775,103 @@ impl<'a> Checker<'a> {

for (sname, fields) in decls {
for (fname, fty) in fields {
let shape = self.shape_of_type(&fty);
match field_borrow_kind(&shape) {
Some("borrow-handle") => {
let msg = format!(
"struct field `{}.{}` has borrow-handle type `{}`",
sname,
fname,
fty.render()
);
let notes = vec![
"fields hold owned values, GC values, ids, or own-handles — never borrows (O12): a stored borrow is the shape that would force lifetime annotations".to_string(),
"process the element inside the loop body (the loan dies at each iteration's last use), or store the index or a copy and re-view through the handle later".to_string(),
];
let span = fty.span();
self.diags.push(
Diagnostic::error("HYB-E3103", self.path.clone(), span, msg)
.with_notes(notes),
);
}
Some("pointer") => {
// O12's own example (`cur &node`): pointer fields are
// borrows — store an id or the value instead.
let msg = format!(
"struct field `{}.{}` has pointer type `{}` — fields never hold borrows",
sname,
fname,
fty.render()
);
let notes = vec![
"a pointer field is a stored borrow (O12): store an id/index and re-borrow at the use site, or store the value".to_string(),
];
let span = fty.span();
self.diags.push(
Diagnostic::error("HYB-E3103", self.path.clone(), span, msg)
.with_notes(notes),
);
self.emit_struct_field_borrow_diag(&sname, &fname, &fty, fty.span());
}
}
}

pub(crate) fn emit_struct_field_borrow_diag(
&mut self,
sname: &str,
fname: &str,
fty: &hy_syntax::TypeExpr,
span: Span,
) {
let shape = self.shape_of_type(fty);
match self.field_borrow_kind(&shape) {
Some("borrow-handle") => {
let msg = format!(
"struct field `{}.{}` has borrow-handle type `{}`",
sname,
fname,
fty.render()
);
let notes = vec![
"fields hold owned values, GC values, ids, or own-handles — never borrows (O12): a stored borrow is the shape that would force lifetime annotations".to_string(),
"process the element inside the loop body (the loan dies at each iteration's last use), or store the index or a copy and re-view through the handle later".to_string(),
];
if !self
.diags
.iter()
.any(|d| d.code == "HYB-E3103" && d.span == span)
{
self.diags.push(
Diagnostic::error("HYB-E3103", self.path.clone(), span, msg)
.with_notes(notes),
);
}
}
Some("pointer") => {
// O12's own example (`cur &node`): pointer fields are
// borrows — store an id or the value instead.
let msg = format!(
"struct field `{}.{}` has pointer type `{}` — fields never hold borrows",
sname,
fname,
fty.render()
);
let notes = vec![
"a pointer field is a stored borrow (O12): store an id/index and re-borrow at the use site, or store the value".to_string(),
];
if !self
.diags
.iter()
.any(|d| d.code == "HYB-E3103" && d.span == span)
{
self.diags.push(
Diagnostic::error("HYB-E3103", self.path.clone(), span, msg)
.with_notes(notes),
);
}
}
_ => {}
}
}

pub(crate) fn field_borrow_kind(&self, shape: &TypeShape) -> Option<&'static str> {
let mut seen = Vec::new();
self.field_borrow_kind_rec(shape, &mut seen)
}

fn field_borrow_kind_rec(
&self,
shape: &TypeShape,
seen: &mut Vec<String>,
) -> Option<&'static str> {
match shape {
TypeShape::BorrowHandle(_) => Some("borrow-handle"),
TypeShape::Pointer(inner) if !matches!(**inner, TypeShape::Atomic) => Some("pointer"),
TypeShape::Slice(elem) | TypeShape::Array(elem) => {
self.field_borrow_kind_rec(elem, seen)
}
TypeShape::Map(k, v) => self
.field_borrow_kind_rec(k, seen)
.or_else(|| self.field_borrow_kind_rec(v, seen)),
TypeShape::Struct(Some(sname)) => {
if seen.iter().any(|s| s == sname) {
return None;
}
seen.push(sname.clone());
let fields = self.struct_fields_for_sname(sname)?;
for (_, fty) in &fields {
let fshape = self.shape_of_type(fty);
if let Some(k) = self.field_borrow_kind_rec(&fshape, seen) {
return Some(k);
}
_ => {}
}
None
}
_ => None,
}
}

Expand Down Expand Up @@ -7849,16 +7905,6 @@ impl<'a> Checker<'a> {
}
}

fn field_borrow_kind(shape: &TypeShape) -> Option<&'static str> {
match shape {
TypeShape::BorrowHandle(_) => Some("borrow-handle"),
TypeShape::Pointer(inner) if !matches!(**inner, TypeShape::Atomic) => Some("pointer"),
TypeShape::Slice(elem) | TypeShape::Array(elem) => field_borrow_kind(elem),
TypeShape::Map(k, v) => field_borrow_kind(k).or_else(|| field_borrow_kind(v)),
_ => None,
}
}

#[cfg(test)]
mod tests {
use crate::{check_file, Verdict};
Expand Down
Loading
Loading