From 2a2b315a806340e0b1217c335c149b64cacd6727 Mon Sep 17 00:00:00 2001 From: Abdullah Azmi Date: Sat, 24 Oct 2026 14:22:47 +0000 Subject: [PATCH] fix(hy-syntax,hy-emit): keep fn-type result parsing on same line and support unnamed/parenthesized signatures (fixes #727) --- compiler/crates/hy-check/src/buildable.rs | 6 +- compiler/crates/hy-check/src/emit_guard.rs | 36 ++- compiler/crates/hy-check/src/loans.rs | 56 ++++- .../tests/call_arity_and_return_types.rs | 31 +++ compiler/crates/hy-emit/src/lib.rs | 28 ++- compiler/crates/hy-emit/src/tests.rs | 47 +++- compiler/crates/hy-syntax/src/parser.rs | 234 ++++++++++++++++-- 7 files changed, 398 insertions(+), 40 deletions(-) diff --git a/compiler/crates/hy-check/src/buildable.rs b/compiler/crates/hy-check/src/buildable.rs index 5324dff96..1316aad7c 100644 --- a/compiler/crates/hy-check/src/buildable.rs +++ b/compiler/crates/hy-check/src/buildable.rs @@ -1735,14 +1735,12 @@ impl<'a> Collector<'a> { OpKind::Chan(Box::new(self.kind_of_ty_with_subst(elem, subst, depth + 1))) } TypeExpr::Option { .. } => OpKind::Unknown, - TypeExpr::Fn { results, .. } => OpKind::Fn( + TypeExpr::Fn { params, results, .. } => OpKind::Fn( results .iter() .map(|r| self.kind_of_ty_with_subst(r, subst, depth + 1)) .collect(), - // A bare fn TYPE does not carry a call signature the kind model - // can trust a count from — keep the arity unknown. - None, + Some(params.len()), ), } } diff --git a/compiler/crates/hy-check/src/emit_guard.rs b/compiler/crates/hy-check/src/emit_guard.rs index 3e5d09c1b..402df9565 100644 --- a/compiler/crates/hy-check/src/emit_guard.rs +++ b/compiler/crates/hy-check/src/emit_guard.rs @@ -640,8 +640,12 @@ impl<'a> Checker<'a> { decl.params.iter().map(|p| (p.name.clone(), p.ty.clone())).collect() }) } else if let Some(b) = self.lookup_binding(st, name) { - if let Some(TypeExpr::Fn { params, .. }) = &b.ty { - Some(params.iter().map(|p| (p.name.clone(), p.ty.clone())).collect()) + if let Some(ty) = &b.ty { + if let TypeExpr::Fn { params, .. } = self.resolve_type_alias(ty) { + Some(params.iter().map(|p| (p.name.clone(), p.ty.clone())).collect()) + } else { + None + } } else { None } @@ -678,15 +682,30 @@ impl<'a> Checker<'a> { }, _ => None, }; - let decl = if let Some(s) = recv_struct { - self.method_decls.get(&(s, method_name.clone())).copied() + let decl = if let Some(ref s) = recv_struct { + self.method_decls.get(&(s.clone(), method_name.clone())).copied() } else { self.method_decls .iter() .find(|((_, m), _)| m == method_name) .map(|(_, d)| *d) }; - decl.map(|d| d.params.iter().map(|p| (p.name.clone(), p.ty.clone())).collect()) + if let Some(d) = decl { + Some(d.params.iter().map(|p| (p.name.clone(), p.ty.clone())).collect()) + } else { + recv_struct + .and_then(|s| self.structs.get(&s)) + .and_then(|fs| fs.iter().find(|(n, _)| n == method_name)) + .and_then(|(_, fty)| match self.resolve_type_alias(fty) { + TypeExpr::Fn { params, .. } => Some( + params + .iter() + .map(|p| (p.name.clone(), p.ty.clone())) + .collect(), + ), + _ => None, + }) + } } } Expr::FnLit { params, .. } => { @@ -707,8 +726,13 @@ impl<'a> Checker<'a> { }; if let Some((p_name, p_ty)) = target { let resolved_ty = self.resolve_type_alias(p_ty); + let label = if p_name.is_empty() { + format!("parameter #{}", i + 1) + } else { + format!("parameter `{}`", p_name) + }; self.check_const_overflow_init( - &format!("parameter `{}`", p_name), + &label, &resolved_ty, &a.value, None, diff --git a/compiler/crates/hy-check/src/loans.rs b/compiler/crates/hy-check/src/loans.rs index cc845c11b..4c2c30569 100644 --- a/compiler/crates/hy-check/src/loans.rs +++ b/compiler/crates/hy-check/src/loans.rs @@ -2084,10 +2084,17 @@ impl<'a> Checker<'a> { match unwrap_parens(callee) { Expr::Ident { name, .. } => { if let Some(b) = self.lookup_binding(st, name) { - if let Some(hy_syntax::TypeExpr::Fn { params, .. }) = &b.ty { - return params - .get(idx) - .is_some_and(|p| matches!(p.ty, hy_syntax::TypeExpr::Pointer { .. })); + if let Some(ty) = &b.ty { + if let hy_syntax::TypeExpr::Fn { params, .. } = self.resolve_type_alias(ty) { + return params + .get(idx) + .is_some_and(|p| { + matches!( + self.resolve_type_alias(&p.ty), + hy_syntax::TypeExpr::Pointer { .. } + ) + }); + } } return false; } @@ -2127,10 +2134,28 @@ impl<'a> Checker<'a> { }, _ => None, }); + if let Some(f) = recv_base + .as_ref() + .and_then(|rb| self.method_decls.get(&(rb.clone(), head.to_string()))) + { + return f + .params + .get(idx) + .is_some_and(|p| matches!(p.ty, hy_syntax::TypeExpr::Pointer { .. })); + } recv_base - .and_then(|rb| self.method_decls.get(&(rb, head.to_string()))) - .and_then(|f| f.params.get(idx)) - .is_some_and(|p| matches!(p.ty, hy_syntax::TypeExpr::Pointer { .. })) + .and_then(|rb| self.structs.get(&rb)) + .and_then(|fs| fs.iter().find(|(n, _)| n == name)) + .and_then(|(_, fty)| match self.resolve_type_alias(fty) { + hy_syntax::TypeExpr::Fn { params, .. } => params.get(idx).map(|p| { + matches!( + self.resolve_type_alias(&p.ty), + hy_syntax::TypeExpr::Pointer { .. } + ) + }), + _ => None, + }) + .unwrap_or(false) } Expr::FnLit { params, .. } => params .get(idx) @@ -6570,7 +6595,9 @@ impl<'a> Checker<'a> { None => (false, true), } } else if let Some(b) = self.lookup_binding(st, name) { - if let Some(hy_syntax::TypeExpr::Fn { params, .. }) = &b.ty { + if let Some(hy_syntax::TypeExpr::Fn { params, .. }) = + b.ty.as_ref().map(|t| self.resolve_type_alias(t)) + { match params.get(i) { Some(p) => { let is_ptr = matches!(p.ty, hy_syntax::TypeExpr::Pointer { .. }) @@ -6608,7 +6635,18 @@ impl<'a> Checker<'a> { }; (false, mutates) } else if matches!(self.shape_of_expr(st, callee), TypeShape::Closure) { - (false, true) + let is_ptr = self + .ty_of_expr(st, callee) + .map(|t| self.resolve_type_alias(&t)) + .and_then(|t| match t { + hy_syntax::TypeExpr::Fn { params, .. } => params.get(i).map(|p| { + matches!(p.ty, hy_syntax::TypeExpr::Pointer { .. }) + || matches!(self.shape_of_type(&p.ty), TypeShape::Pointer(_)) + }), + _ => None, + }) + .unwrap_or(false); + (is_ptr, true) } else { let recv_base = match self.shape_of_expr(st, base) { TypeShape::Struct(Some(s)) => Some(s), diff --git a/compiler/crates/hy-check/tests/call_arity_and_return_types.rs b/compiler/crates/hy-check/tests/call_arity_and_return_types.rs index 5041d35ec..645882dd0 100644 --- a/compiler/crates/hy-check/tests/call_arity_and_return_types.rs +++ b/compiler/crates/hy-check/tests/call_arity_and_return_types.rs @@ -308,3 +308,34 @@ fn call_argument_types_stay_accepted() { ); } +#[test] +fn fn_type_local_and_struct_field_soundness_issue_727() { + assert_ok( + "package main\n\nimport \"fmt\"\n\nfn main() {\n var cb fn() int\n fmt.println(cb == nil)\n}\n", + ); + assert_ok("package main\n\nfn main() {\n var f fn(int) int\n _ = f(1)\n}\n"); + assert_ok( + "package main\n\nstruct handler {\n on_event fn(int) bool\n name string\n}\n\nfn main() {\n h := handler{name: \"click\"}\n _ = h.name\n}\n", + ); + assert_code( + "package main\n\nfn main() {\n var f fn(int) int\n _ = f(1, 2)\n}\n", + "HYB-E1002", + ); + assert_code( + "package main\n\nfn main() {\n var f fn(u8) int\n _ = f(999)\n}\n", + "HYB-E6002", + ); + assert_code( + "package main\n\ntype cb = fn(u8) int\n\nfn main() {\n var f cb\n _ = f(999)\n}\n", + "HYB-E6002", + ); + assert_code( + "package main\n\nstruct handler {\n on_event fn(u8) bool\n}\n\nfn main() {\n var h handler\n _ = h.on_event(999)\n}\n", + "HYB-E6002", + ); + assert!( + codes("package main\n\nfn main() {\n var f fn([no_such_const]int) int\n _ = f\n}\n") + .contains(&"HYB-E1001".to_string()) + ); +} + diff --git a/compiler/crates/hy-emit/src/lib.rs b/compiler/crates/hy-emit/src/lib.rs index 95037090c..c14620bd9 100644 --- a/compiler/crates/hy-emit/src/lib.rs +++ b/compiler/crates/hy-emit/src/lib.rs @@ -1737,9 +1737,24 @@ impl Emitter { TypeExpr::Fn { params, results, .. } => { + let all_named = params.iter().all(|p| { + !p.name.is_empty() + && !matches!(&p.ty, TypeExpr::Named { name, .. } if name.is_empty()) + }); let ps = params .iter() - .map(|p| format!("{} {}", p.name, self.type_text(&p.ty))) + .map(|p| { + let t = self.type_text(&p.ty); + if p.name.is_empty() { + t + } else if t.is_empty() { + self.go_type_name(&p.name) + } else if all_named { + format!("{} {}", p.name, t) + } else { + t + } + }) .collect::>() .join(", "); let rs = results @@ -1747,10 +1762,10 @@ impl Emitter { .map(|t| self.type_text(t)) .collect::>() .join(", "); - if rs.is_empty() { - format!("func({})", ps) - } else { - format!("func({}) {}", ps, rs) + match results.len() { + 0 => format!("func({})", ps), + 1 => format!("func({}) {}", ps, rs), + _ => format!("func({}) ({})", ps, rs), } } } @@ -1910,7 +1925,8 @@ impl Emitter { TypeExpr::Slice { .. } | TypeExpr::Map { .. } | TypeExpr::Chan { .. } - | TypeExpr::Pointer { .. } => "nil".into(), + | TypeExpr::Pointer { .. } + | TypeExpr::Fn { .. } => "nil".into(), TypeExpr::Option { .. } => format!("{}{{}}", self.type_text(t)), _ => format!("*new({})", self.type_text(t)), } diff --git a/compiler/crates/hy-emit/src/tests.rs b/compiler/crates/hy-emit/src/tests.rs index c05bb5e4f..32f684ac7 100644 --- a/compiler/crates/hy-emit/src/tests.rs +++ b/compiler/crates/hy-emit/src/tests.rs @@ -2943,7 +2943,6 @@ fn m4_initialism_mapping_for_foreign_go_names_issue_658() { assert!(go.contains("strings.ToValidUTF8(\"a\", \"b\")"), "expected strings.ToValidUTF8, got:\n{}", go); } - #[test] fn package_mode_export_normalization_collision_issue_1421() { let check_collision = |src: &str, expected_msg_part: &str| { @@ -3032,3 +3031,49 @@ fn main() {} assert!(go6.contains("func read_url() int {")); assert!(go6.contains("func read_u_r_l() int {")); } + +#[test] +fn fn_type_local_and_struct_field_emit_valid_go_issue_727() { + let src = r#" +package main + +import "fmt" + +struct handler { + on_event fn(int) bool + name string +} + +fn main() { + var cb fn() int + fmt.println(cb == nil) + var f fn(u8) (int, error) + _ = f + var mixed fn(x int, string) bool + _ = mixed + h := handler{on_event: fn(x int) bool { return x > 0 }, name: "click"} + fmt.println(h.name, h.on_event(1)) +} +"#; + let go = emit(src); + assert!( + go.contains("var cb func() int\n\tfmt.Println(cb == nil)"), + "var cb fn() int must not absorb following statement:\n{}", + go + ); + assert!( + go.contains("var f func(uint8) (int, error)"), + "unnamed primitive param and multi-result fn type must emit valid Go:\n{}", + go + ); + assert!( + go.contains("var mixed func(int, string) bool"), + "mixed named and unnamed fn-type params must emit unnamed Go params:\n{}", + go + ); + assert!( + go.contains("on_event func(int) bool") && go.contains("name") && go.contains("h.name"), + "struct fields after fn-typed field must be preserved:\n{}", + go + ); +} diff --git a/compiler/crates/hy-syntax/src/parser.rs b/compiler/crates/hy-syntax/src/parser.rs index b2b0c45b6..4cea5b106 100644 --- a/compiler/crates/hy-syntax/src/parser.rs +++ b/compiler/crates/hy-syntax/src/parser.rs @@ -608,6 +608,62 @@ impl Parser { Ok(params) } + /// Parameter list inside a `fn(...)` type expression: parameters may be + /// unnamed (`fn(int, *string) bool`) or named (`fn(a int, b string) bool`). + fn parse_fn_type_params(&mut self) -> PResult> { + let mut params = Vec::new(); + while !self.at_punct(")") && !self.at_eof() { + let pstart = self.pos; + let param = if matches!(self.cur(), Tok::Ident(_)) { + let saved_pos = self.pos; + let saved_diags = self.diags.len(); + let unnamed = if let Ok(ty) = self.parse_type() { + if self.at_punct(",") || self.at_punct(")") { + Some(Param { + name: String::new(), + ty, + span: self.span_from(pstart), + }) + } else { + None + } + } else { + None + }; + match unnamed { + Some(p) => p, + None => { + self.pos = saved_pos; + self.diags.truncate(saved_diags); + let name = self.expect_ident()?; + let ty = self.parse_type()?; + Param { + name, + ty, + span: self.span_from(pstart), + } + } + } + } else { + let ty = self.parse_type()?; + Param { + name: String::new(), + ty, + span: self.span_from(pstart), + } + }; + params.push(param); + if !self.at_punct(",") && !self.at_punct(")") { + return self.err(format!( + "expected ',' or ')' in parameter list, found {}", + self.describe_cur() + )); + } + self.eat_punct(","); + } + Ok(params) + } + /// Result: bare type, or parenthesized list with optional names /// (`(total int, err error)` / `(int, error)` / `(code int)`). fn parse_results(&mut self) -> PResult> { @@ -615,19 +671,36 @@ impl Parser { self.bump(); let mut results = Vec::new(); while !self.at_punct(")") && !self.at_eof() { - let named = - matches!(self.cur(), Tok::Ident(_)) && matches!(self.nth(1), Tok::Ident(_)); - if named { - let name = self.expect_ident()?; - let ty = self.parse_type()?; - results.push(FnResult { - name: Some(name), - ty, - }); + let result = if matches!(self.cur(), Tok::Ident(_)) { + let saved_pos = self.pos; + let saved_diags = self.diags.len(); + let unnamed = if let Ok(ty) = self.parse_type() { + if self.at_punct(",") || self.at_punct(")") { + Some(FnResult { name: None, ty }) + } else { + None + } + } else { + None + }; + match unnamed { + Some(r) => r, + None => { + self.pos = saved_pos; + self.diags.truncate(saved_diags); + let name = self.expect_ident()?; + let ty = self.parse_type()?; + FnResult { + name: Some(name), + ty, + } + } + } } else { let ty = self.parse_type()?; - results.push(FnResult { name: None, ty }); - } + FnResult { name: None, ty } + }; + results.push(result); if !self.at_punct(",") && !self.at_punct(")") { return self.err("expected ',' or ')' in result list"); } @@ -731,11 +804,17 @@ impl Parser { Tok::Kw("fn") => { self.bump(); self.expect_punct("(")?; - let params = self.parse_params()?; + let params = self.parse_fn_type_params()?; self.expect_punct(")")?; let mut results = Vec::new(); - while self.is_type_start() { - results.push(self.parse_type()?); + if self.same_line_as_prev() && self.at_punct("(") { + for r in self.parse_results()? { + results.push(r.ty); + } + } else { + while self.same_line_as_prev() && self.is_type_start() { + results.push(self.parse_type()?); + } } TypeExpr::Fn { params, @@ -4383,5 +4462,132 @@ fn main() { assert!(r.is_err(), "leading dot on new line must not fold into statement"); } + #[test] + fn fn_type_does_not_absorb_following_line_as_result_type_issue_727() { + let src = r#" +package main + +import "fmt" + +struct handler { + on_event fn(int) bool + name string +} + +fn main() { + var cb fn() int + fmt.println(cb == nil) + var f fn(int) int + f(1) + var void_cb fn() + x := 1 + _ = x +} +"#; + let f = parse_src(src); + let st = f + .decls + .iter() + .find_map(|d| match d { + Decl::Struct(s) if s.name == "handler" => Some(s), + _ => None, + }) + .expect("handler struct"); + assert_eq!( + st.fields.len(), + 2, + "struct fields after fn-typed field must not be absorbed: {:?}", + st.fields + ); + assert_eq!(st.fields[0].name, "on_event"); + assert_eq!(st.fields[1].name, "name"); + match &st.fields[0].ty { + TypeExpr::Fn { + params, results, .. + } => { + assert_eq!(params.len(), 1); + assert_eq!(params[0].name, ""); + assert_eq!(params[0].ty.render(), "int"); + assert_eq!(results.len(), 1); + assert_eq!(results[0].render(), "bool"); + } + other => panic!("expected TypeExpr::Fn, got {:?}", other), + } + + let main_fn = f + .decls + .iter() + .find_map(|d| match d { + Decl::Fn(fn_decl) if fn_decl.name == "main" => Some(fn_decl), + _ => None, + }) + .expect("main fn"); + let body = main_fn.body.as_ref().expect("main body"); + assert_eq!( + body.stmts.len(), + 7, + "statements following var fn(...) must not be swallowed: {:?}", + body.stmts + ); + match &body.stmts[0] { + Stmt::Local(LocalDecl { + ty: Some(TypeExpr::Fn { results, .. }), + .. + }) => { + assert_eq!(results.len(), 1); + assert_eq!(results[0].render(), "int"); + } + other => panic!("expected var cb fn() int, got {:?}", other), + } + assert!(matches!( + &body.stmts[1], + Stmt::Expr { + expr: Expr::Call { .. }, + .. + } + )); + assert!(matches!( + &body.stmts[3], + Stmt::Expr { + expr: Expr::Call { .. }, + .. + } + )); + } + + #[test] + fn fn_type_supports_unnamed_complex_params_and_parenthesized_results_issue_727() { + let src = r#" +package main + +fn main() { + var f fn(*int, []string, map[string]int) (int, error) + _ = f +} +"#; + let f = parse_src(src); + let main_fn = f.fns().next().expect("main"); + let body = main_fn.body.as_ref().expect("body"); + match &body.stmts[0] { + Stmt::Local(LocalDecl { + ty: + Some(TypeExpr::Fn { + params, results, .. + }), + .. + }) => { + assert_eq!(params.len(), 3); + assert!(params.iter().all(|p| p.name.is_empty())); + assert_eq!(params[0].ty.render(), "*int"); + assert_eq!(params[1].ty.render(), "[]string"); + assert_eq!(params[2].ty.render(), "map[string]int"); + assert_eq!(results.len(), 2); + assert_eq!(results[0].render(), "int"); + assert_eq!(results[1].render(), "error"); + } + other => panic!("expected fn type local, got {:?}", other), + } + } + }