From 60eb65be680e804c0c3da72cc9309f2d65aaa1c3 Mon Sep 17 00:00:00 2001 From: Cormac Relf Date: Sun, 16 Nov 2025 01:15:41 +1100 Subject: [PATCH 01/10] Remove vestigial option wrapping in BindingsCollect::visit_def --- starlark-rust/starlark/src/typing/bindings.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/starlark-rust/starlark/src/typing/bindings.rs b/starlark-rust/starlark/src/typing/bindings.rs index e7ff9b2f67b8b..12c99b7c487ab 100644 --- a/starlark-rust/starlark/src/typing/bindings.rs +++ b/starlark-rust/starlark/src/typing/bindings.rs @@ -231,7 +231,7 @@ impl<'a, 'b> BindingsCollect<'a, 'b> { let name = &p.node.ident; let ty = p.node.ty; let ty = Self::resolve_ty_opt(ty, typecheck_mode, codemap)?; - let name_ty = match &p.node.kind { + let ty = match &p.node.kind { DefParamKind::Regular(mode, default_value) => { let required = match default_value.is_some() { true => ParamIsRequired::No, @@ -256,25 +256,23 @@ impl<'a, 'b> BindingsCollect<'a, 'b> { )); } } - Some((name, ty)) + ty } DefParamKind::Args => { // There is the type we require people calling us use (usually any) // and then separately the type we are when we are running (always tuple) args = Some(ty.dupe()); - Some((name, Ty::basic(TyBasic::Tuple(TyTuple::Of(ArcTy::new(ty)))))) + Ty::basic(TyBasic::Tuple(TyTuple::Of(ArcTy::new(ty)))) } DefParamKind::Kwargs => { let var_ty = Ty::dict(Ty::string(), ty.clone()); kwargs = Some(ty.dupe()); - Some((name, var_ty)) + var_ty } }; - if let Some((name, ty)) = name_ty { - self.bindings - .types - .insert(name.resolved_binding_id(codemap)?, ty); - } + self.bindings + .types + .insert(name.resolved_binding_id(codemap)?, ty); } let params2 = ParamSpec::new_parts(pos_only, pos_or_named, args, named_only, kwargs) .map_err(|e| InternalError::from_error(e, def.signature_span(), codemap))?; From 2b976732a3d1cde7135a7568e9288102ccd2c543 Mon Sep 17 00:00:00 2001 From: Cormac Relf Date: Sun, 16 Nov 2025 21:15:38 +1100 Subject: [PATCH 02/10] Refactor insertion of binding types into a function We will later use this to disallow duplicate type annotations. That's why we include error_span, etc. --- starlark-rust/starlark/src/typing/bindings.rs | 38 +++++++++++++++---- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/starlark-rust/starlark/src/typing/bindings.rs b/starlark-rust/starlark/src/typing/bindings.rs index 12c99b7c487ab..b83528cb6e1ed 100644 --- a/starlark-rust/starlark/src/typing/bindings.rs +++ b/starlark-rust/starlark/src/typing/bindings.rs @@ -39,6 +39,7 @@ use crate::codemap::Span; use crate::codemap::Spanned; use crate::eval::compiler::scope::BindingId; use crate::eval::compiler::scope::ResolvedIdent; +use crate::eval::compiler::scope::payload::CstAssignIdent; use crate::eval::compiler::scope::payload::CstAssignIdentExt; use crate::eval::compiler::scope::payload::CstAssignTarget; use crate::eval::compiler::scope::payload::CstExpr; @@ -206,6 +207,21 @@ impl<'a, 'b> BindingsCollect<'a, 'b> { } } + /// Insert an explicit type for the binding. + fn type_annotation( + &mut self, + binding: &CstAssignIdent, + ty: Ty, + _error_span: Span, + codemap: &CodeMap, + ) -> Result<(), InternalError> { + let resolved_id = binding.resolved_binding_id(codemap)?; + // FIXME: This could be duplicated if you declare the type of a variable twice. + // We would only see the second one. + self.bindings.types.insert(resolved_id, ty); + Ok(()) + } + fn visit_def( &mut self, def: &'a DefP, @@ -277,10 +293,20 @@ impl<'a, 'b> BindingsCollect<'a, 'b> { let params2 = ParamSpec::new_parts(pos_only, pos_or_named, args, named_only, kwargs) .map_err(|e| InternalError::from_error(e, def.signature_span(), codemap))?; let ret_ty = Self::resolve_ty_opt(return_type.as_deref(), typecheck_mode, codemap)?; - self.bindings.types.insert( - name.resolved_binding_id(codemap)?, + + // Defining a function is like an annotated assignment + // + // foo: Function[...] = lambda x, y: ... + // + // (even if there are no type annotations, because even just having parameters + // is a kind of type annotation). + // + self.type_annotation( + name, Ty::function(params2, ret_ty.clone()), - ); + name.span, + codemap, + )?; def.visit_children_err(|x| self.visit(x, &ret_ty, typecheck_mode, codemap))?; Ok(()) } @@ -301,11 +327,7 @@ impl<'a, 'b> BindingsCollect<'a, 'b> { .check_type .push((ty.span, Some(rhs), ty2.clone())); if let AssignTargetP::Identifier(id) = &**lhs { - // FIXME: This could be duplicated if you declare the type of a variable twice, - // we would only see the second one. - self.bindings - .types - .insert(id.resolved_binding_id(codemap)?, ty2); + self.type_annotation(id, ty2, lhs.span.merge(ty.span), codemap)?; } } self.assign(lhs, BindExpr::Expr(rhs), codemap)? From 98873d971adb9c9c1dc070893de6db210b0be09e Mon Sep 17 00:00:00 2001 From: Cormac Relf Date: Wed, 19 Nov 2025 19:37:11 +1100 Subject: [PATCH 03/10] Add DefP::visit_header For use in BindingsCollect. --- .../starlark_syntax/src/syntax/uniplate.rs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/starlark-rust/starlark_syntax/src/syntax/uniplate.rs b/starlark-rust/starlark_syntax/src/syntax/uniplate.rs index c1df16dceb79a..a68a64dba2402 100644 --- a/starlark-rust/starlark_syntax/src/syntax/uniplate.rs +++ b/starlark-rust/starlark_syntax/src/syntax/uniplate.rs @@ -86,6 +86,36 @@ impl DefP

{ f(Visit::Stmt(body)); } + /// Visit the parameters and return type, but not the body + pub fn visit_header_err<'a, E>( + &'a self, + mut f: impl FnMut(Visit<'a, P>) -> Result<(), E>, + ) -> Result<(), E> { + let DefP { + name: _, + params, + return_type, + body: _, + payload: _, + } = self; + let mut result = Ok(()); + params.iter().for_each(|x| { + x.visit_expr(|x| { + if result.is_ok() { + result = f(Visit::Expr(x)); + } + }) + }); + return_type.iter().for_each(|x| { + x.visit_expr(|x| { + if result.is_ok() { + result = f(Visit::Expr(x)); + } + }) + }); + result + } + pub fn visit_children_err<'a, E>( &'a self, mut f: impl FnMut(Visit<'a, P>) -> Result<(), E>, From 188aa5a7eaceb1bc16662189ca80c19f737f2669 Mon Sep 17 00:00:00 2001 From: Cormac Relf Date: Wed, 19 Nov 2025 21:50:14 +1100 Subject: [PATCH 04/10] Add TypingOrInternalError::into_eval_exception --- starlark-rust/starlark/src/typing/error.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/starlark-rust/starlark/src/typing/error.rs b/starlark-rust/starlark/src/typing/error.rs index 994e8ddb0f441..879b1386164d1 100644 --- a/starlark-rust/starlark/src/typing/error.rs +++ b/starlark-rust/starlark/src/typing/error.rs @@ -138,6 +138,16 @@ impl From for TypingOrInternalError { } } +impl TypingOrInternalError { + #[cold] + pub(crate) fn into_eval_exception(self) -> EvalException { + match self { + Self::Typing(e) => e.into_eval_exception(), + Self::Internal(e) => e.into_eval_exception(), + } + } +} + /// Like [`TypingOrInternalError`], but without context on the typing variant. pub enum TypingNoContextOrInternalError { /// Types are not compatible. From f751e7bffe29d2c2e8f4c8eba4e46463afc72e2b Mon Sep 17 00:00:00 2001 From: Cormac Relf Date: Mon, 17 Nov 2025 21:05:21 +1100 Subject: [PATCH 05/10] Add TypingOrInternalError::into_error --- starlark-rust/starlark/src/typing/error.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/starlark-rust/starlark/src/typing/error.rs b/starlark-rust/starlark/src/typing/error.rs index 879b1386164d1..bfb460ba85851 100644 --- a/starlark-rust/starlark/src/typing/error.rs +++ b/starlark-rust/starlark/src/typing/error.rs @@ -146,6 +146,14 @@ impl TypingOrInternalError { Self::Internal(e) => e.into_eval_exception(), } } + + #[cold] + pub(crate) fn into_error(self) -> crate::Error { + match self { + Self::Typing(e) => e.into_error(), + Self::Internal(e) => e.into_error(), + } + } } /// Like [`TypingOrInternalError`], but without context on the typing variant. From fc1a8609fc23911b390b319fe6cbea12b7b75ddc Mon Sep 17 00:00:00 2001 From: Cormac Relf Date: Sun, 16 Nov 2025 23:47:23 +1100 Subject: [PATCH 06/10] Type check every function and top-level scope separately Previously, we gathered and solved bindings for every top-level def, and not for a module's root bindings. This was a deliberate limitation for typechecker performance reasons, introduced in D48752028/D48752027 back in 2023. This completes the work described in those diffs, which said ideally: 1. typecheck top-level assignments 2. then evaluate defs This does one better and typechecks even nested defs individually. This achieves the goal of reducing the iterations needed in solve_bindings by passing the smallest possible quantity of bindings to solve_bindings at any one time. --- starlark-rust/starlark/src/typing/bindings.rs | 49 +++++++- .../starlark/src/typing/typecheck.rs | 105 ++++++++++++++++++ 2 files changed, 150 insertions(+), 4 deletions(-) diff --git a/starlark-rust/starlark/src/typing/bindings.rs b/starlark-rust/starlark/src/typing/bindings.rs index b83528cb6e1ed..bea1abe399f49 100644 --- a/starlark-rust/starlark/src/typing/bindings.rs +++ b/starlark-rust/starlark/src/typing/bindings.rs @@ -101,12 +101,43 @@ pub(crate) struct Bindings<'a> { pub(crate) check_type: Vec<(Span, Option<&'a CstExpr>, Ty)>, } +/// Basically a child `def`. +pub(crate) struct ChildDef<'a> { + pub(crate) body: &'a CstStmt, + pub(crate) return_type: Ty, + pub(crate) param_types: HashMap, +} + pub(crate) struct BindingsCollect<'a, 'b> { pub(crate) bindings: Bindings<'a>, pub(crate) approximations: &'b mut Vec, + pub(crate) children_sink: Option<&'b mut Vec>>, } impl<'a, 'b> BindingsCollect<'a, 'b> { + /// Collect all the assignments to variables in a scope. + /// + /// This function only fails on internal errors. + pub(crate) fn collect_scope( + scope: &'a CstStmt, + return_type: &Ty, + visible: &HashMap, + typecheck_mode: TypecheckMode, + codemap: &CodeMap, + approximations: &'b mut Vec, + children_sink: &'b mut Vec>, + ) -> Result, InternalError> { + let mut res = BindingsCollect { + bindings: Bindings::default(), + approximations, + children_sink: Some(children_sink), + }; + res.bindings.types = visible.clone(); + + res.visit(Visit::Stmt(scope), return_type, typecheck_mode, codemap)?; + Ok(res.bindings) + } + /// Collect all the assignments to variables. /// /// This function only fails on internal errors. @@ -119,6 +150,7 @@ impl<'a, 'b> BindingsCollect<'a, 'b> { let mut res = BindingsCollect { bindings: Bindings::default(), approximations, + children_sink: None, }; res.visit(Visit::Stmt(x), &Ty::any(), typecheck_mode, codemap)?; @@ -242,6 +274,7 @@ impl<'a, 'b> BindingsCollect<'a, 'b> { let mut args = None; let mut named_only = Vec::new(); let mut kwargs = None; + let mut param_types = HashMap::new(); for p in params { let name = &p.node.ident; @@ -286,9 +319,7 @@ impl<'a, 'b> BindingsCollect<'a, 'b> { var_ty } }; - self.bindings - .types - .insert(name.resolved_binding_id(codemap)?, ty); + param_types.insert(name.resolved_binding_id(codemap)?, ty); } let params2 = ParamSpec::new_parts(pos_only, pos_or_named, args, named_only, kwargs) .map_err(|e| InternalError::from_error(e, def.signature_span(), codemap))?; @@ -307,7 +338,17 @@ impl<'a, 'b> BindingsCollect<'a, 'b> { name.span, codemap, )?; - def.visit_children_err(|x| self.visit(x, &ret_ty, typecheck_mode, codemap))?; + + def.visit_header_err(|x| self.visit(x, &ret_ty, typecheck_mode, codemap))?; + + // Function body just gets added to the queue. + if let Some(sink) = &mut self.children_sink { + sink.push(ChildDef { + body: &def.body, + param_types, + return_type: ret_ty, + }); + } Ok(()) } diff --git a/starlark-rust/starlark/src/typing/typecheck.rs b/starlark-rust/starlark/src/typing/typecheck.rs index 955dd493d80ff..e523455f0b687 100644 --- a/starlark-rust/starlark/src/typing/typecheck.rs +++ b/starlark-rust/starlark/src/typing/typecheck.rs @@ -46,6 +46,7 @@ use crate::typing::bindings::BindingsCollect; use crate::typing::ctx::TypingContext; use crate::typing::error::InternalError; use crate::typing::error::TypingError; +use crate::typing::error::TypingOrInternalError; use crate::typing::fill_types_for_lint::ModuleVarTypes; use crate::typing::fill_types_for_lint::fill_types_for_lint_typechecker; use crate::typing::interface::Interface; @@ -55,6 +56,110 @@ use crate::typing::ty::Approximation; use crate::typing::ty::Ty; use crate::values::FrozenHeap; +/// Recursive function type-checker. +/// +/// You can call `solve_bindings` on as big or as little input as you like, but it +/// is O(n * m) where n is the number of expressions assigned to bindings, and m is +/// a measure of complexity of the assignments (`a=1; b=a; b=""; c=b; a=c` is +/// deliberately complex). If you lump all bindings in an entire module together and +/// solve them all at once, `m` is set to the most complex function and `n` is large. +/// +/// Fortunately, because of variable shadowing and no `global` keyword, we only ever +/// need to look at a function body to determine the types of its local bindings from +/// the assignments made to it. +/// +/// ```python +/// x = 5 +/// def child(): +/// x = "string" # completely different binding, no relation to outer `x` +/// ``` +/// +/// In some circumstances child scopes could influence parent scopes via mutation +/// methods, but it's not a big loss if we don't infer in these cases. +/// +/// ```python +/// xs = list() +/// def child(): +/// xs.push(5) # could influence type of xs, but we will not support this +/// ``` +/// +/// So we can solve bindings for every scope individually. That's what this does. +/// As for ordering, a parent scope is finished and solved before we check any of +/// its child scopes, so child scopes have access to parent binding solutions. +/// +/// ```python +/// x = 5 +/// def child(): +/// y = x # y solves to `int | str` +/// x = "string" +/// ``` +/// +pub(crate) struct TypeChecker<'a> { + pub(crate) oracle: TypingOracleCtx<'a>, + pub(crate) typecheck_mode: TypecheckMode, + pub(crate) module_var_types: &'a ModuleVarTypes, + pub(crate) approximations: &'a mut Vec, + pub(crate) all_solved_types: HashMap, +} + +impl<'a> TypeChecker<'a> { + fn codemap(&self) -> &'a CodeMap { + self.oracle.codemap + } + + /// Typecheck an entire module. + /// + /// To just immediately return on encountering a type error, pass `&mut Err` as the error + /// handler. To collect errors and continue, be sure to return Ok from the error handler. + pub(crate) fn check_module_scope( + &mut self, + module: &CstStmt, + eh: &mut dyn FnMut(TypingError) -> Result<(), TypingError>, + ) -> Result<(), TypingOrInternalError> { + self.check_scope(module, &Ty::any(), &HashMap::default(), eh) + } + + /// Recursive scope check. Checks the scope's bindings, and then all child defs. + pub(crate) fn check_scope( + &mut self, + body: &CstStmt, + return_type: &Ty, + visible: &HashMap, + eh: &mut dyn FnMut(TypingError) -> Result<(), TypingError>, + ) -> Result<(), TypingOrInternalError> { + let mut children = Vec::new(); + let bindings = BindingsCollect::collect_scope( + body, + return_type, + visible, + self.typecheck_mode, + self.codemap(), + self.approximations, + &mut children, + )?; + let (errors, solved, mut approx) = + solve_bindings(bindings, self.oracle, self.module_var_types)?; + + self.approximations.append(&mut approx); + + for error in errors { + eh(error)?; + } + + // Save all solved types to the output + let solved_copy = solved.iter().map(|(&b, ty)| (b, ty.dupe())); + self.all_solved_types.extend(solved_copy.clone()); + + for child in children { + let mut child_visible = visible.clone(); + child_visible.extend(solved_copy.clone()); + child_visible.extend(child.param_types); + self.check_scope(child.body, &child.return_type, &child_visible, eh)?; + } + Ok(()) + } +} + // Things which are None in the map have type void - they are never constructed pub(crate) fn solve_bindings( bindings: Bindings, From 10649afc4aa0668e091b23587cd6744385fe3ed7 Mon Sep 17 00:00:00 2001 From: Cormac Relf Date: Sun, 16 Nov 2025 23:47:23 +1100 Subject: [PATCH 07/10] Use recursive function type checker in the compiler --- .../starlark/src/eval/compiler/module.rs | 44 ++++++++----------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/starlark-rust/starlark/src/eval/compiler/module.rs b/starlark-rust/starlark/src/eval/compiler/module.rs index dce7c13a42c41..35c83f3f16c85 100644 --- a/starlark-rust/starlark/src/eval/compiler/module.rs +++ b/starlark-rust/starlark/src/eval/compiler/module.rs @@ -17,6 +17,8 @@ //! Compile and evaluate module top-level statements. +use std::collections::HashMap; + use starlark_syntax::eval_exception::EvalException; use starlark_syntax::syntax::ast::LoadP; use starlark_syntax::syntax::ast::StmtP; @@ -36,11 +38,9 @@ use crate::eval::runtime::frame_span::FrameSpan; use crate::eval::runtime::frozen_file_span::FrozenFileSpan; use crate::typing::Ty; use crate::typing::TypingOracleCtx; -use crate::typing::bindings::BindingsCollect; -use crate::typing::error::InternalError; use crate::typing::fill_types_for_lint::ModuleVarTypes; use crate::typing::mode::TypecheckMode; -use crate::typing::typecheck::solve_bindings; +use crate::typing::typecheck::TypeChecker; use crate::values::FrozenRef; use crate::values::FrozenStringValue; use crate::values::Value; @@ -162,12 +162,12 @@ impl<'v> Compiler<'v, '_, '_, '_> { } } - self.typecheck(&mut stmts)?; + self.typecheck(stmt)?; Ok(last) } - fn typecheck(&mut self, stmts: &mut [&mut CstStmt]) -> Result<(), EvalException> { + fn typecheck(&mut self, stmt: &CstStmt) -> Result<(), EvalException> { let typecheck = self.eval.static_typechecking || self.typecheck; if !typecheck { return Ok(()); @@ -177,27 +177,19 @@ impl<'v> Compiler<'v, '_, '_, '_> { codemap: &self.codemap, }; let module_var_types = self.mk_module_var_types(); - for top in stmts.iter_mut() { - if let StmtP::Def(_) = &mut top.node { - let BindingsCollect { bindings, .. } = BindingsCollect::collect_one( - top, - TypecheckMode::Compiler, - &self.codemap, - &mut Vec::new(), - ) - .map_err(InternalError::into_eval_exception)?; - let (errors, ..) = match solve_bindings(bindings, oracle, &module_var_types) { - Ok(x) => x, - Err(e) => return Err(e.into_eval_exception()), - }; - - if let Some(error) = errors.into_iter().next() { - return Err(error.into_eval_exception()); - } - } - } - - Ok(()) + let mut approximations = Vec::new(); + let mut checker = TypeChecker { + oracle, + typecheck_mode: TypecheckMode::Compiler, + module_var_types: &module_var_types, + approximations: &mut approximations, + all_solved_types: HashMap::new(), + }; + // Just immediately return on any type error + let mut error_handler = Err; + checker + .check_module_scope(stmt, &mut error_handler) + .map_err(|e| e.into_eval_exception()) } fn mk_module_var_types(&self) -> ModuleVarTypes { From 0d15010873aa273dfa403417b5070fdf0b7d149c Mon Sep 17 00:00:00 2001 From: Cormac Relf Date: Mon, 17 Nov 2025 19:56:44 +1100 Subject: [PATCH 08/10] Use recursive function type checker in AstModule's typechecker too --- .../starlark/src/typing/typecheck.rs | 88 ++++++++----------- 1 file changed, 36 insertions(+), 52 deletions(-) diff --git a/starlark-rust/starlark/src/typing/typecheck.rs b/starlark-rust/starlark/src/typing/typecheck.rs index e523455f0b687..38f3d7df35430 100644 --- a/starlark-rust/starlark/src/typing/typecheck.rs +++ b/starlark-rust/starlark/src/typing/typecheck.rs @@ -23,7 +23,6 @@ use std::fmt::Display; use dupe::Dupe; use starlark_map::unordered_map::UnorderedMap; use starlark_syntax::slice_vec_ext::VecExt; -use starlark_syntax::syntax::ast::StmtP; use starlark_syntax::syntax::ast::Visibility; use starlark_syntax::syntax::module::AstModuleFields; use starlark_syntax::syntax::top_level_stmts::top_level_stmts_mut; @@ -321,12 +320,12 @@ impl AstModuleTypecheck for AstModule { let scope_errors = scope_errors.into_map(TypingError::from_eval_exception); // We don't really need to properly unpack top-level statements, // but make it safe against future changes. - let mut cst: Vec<&mut CstStmt> = top_level_stmts_mut(&mut cst); + let mut cst_toplevel: Vec<&mut CstStmt> = top_level_stmts_mut(&mut cst); let oracle = TypingOracleCtx { codemap: &codemap }; let mut approximations = Vec::new(); let (fill_types_errors, module_var_types) = match fill_types_for_lint_typechecker( - &mut cst, + &mut cst_toplevel, oracle, &scope_data, &mut approximations, @@ -346,58 +345,43 @@ impl AstModuleTypecheck for AstModule { }; let mut typemap = UnorderedMap::new(); + let oracle = TypingOracleCtx { codemap: &codemap }; + let mut type_checker = TypeChecker { + oracle, + typecheck_mode: TypecheckMode::Lint, + module_var_types: &module_var_types, + approximations: &mut approximations, + all_solved_types: HashMap::new(), + }; let mut all_solve_errors = Vec::new(); - for top in cst.iter_mut() { - if let StmtP::Def(_) = &mut top.node { - let bindings = match BindingsCollect::collect_one( - top, - TypecheckMode::Lint, - &codemap, - &mut approximations, - ) { - Ok(bindings) => bindings, - Err(e) => { - return ( - vec![InternalError::into_error(e)], - TypeMap { - codemap, - bindings: UnorderedMap::new(), - }, - Interface::default(), - Vec::new(), - ); - } - }; - let (solve_errors, types, solve_approximations) = - match solve_bindings(bindings.bindings, oracle, &module_var_types) { - Ok(x) => x, - Err(e) => { - return ( - vec![e.into_error()], - TypeMap { - codemap, - bindings: UnorderedMap::new(), - }, - Interface::default(), - Vec::new(), - ); - } - }; - - all_solve_errors.extend(solve_errors); - approximations.extend(solve_approximations); + let mut error_handler = |error| { + all_solve_errors.push(error); + // continue checking + Ok(()) + }; + if let Err(unrecoverable) = type_checker.check_module_scope(&cst, &mut error_handler) { + // This is generally an internal error, since most typing errors are handled + // by error_handler. Except some type errors that can't (yet?) be recovered. + return ( + vec![unrecoverable.into_error()], + TypeMap { + codemap, + bindings: UnorderedMap::new(), + }, + Interface::default(), + Vec::new(), + ); + } - for (id, ty) in &types { - let binding = scope_data.get_binding(*id); - let name = binding.name.as_str().to_owned(); - let span = match binding.source { - BindingSource::Source(span) => span, - BindingSource::FromModule => Span::default(), - }; - typemap.insert(*id, (name, span, ty.clone())); - } - } + for (id, ty) in &type_checker.all_solved_types { + let binding = scope_data.get_binding(*id); + let name = binding.name.as_str().to_owned(); + let span = match binding.source { + BindingSource::Source(span) => span, + BindingSource::FromModule => Span::default(), + }; + typemap.insert(*id, (name, span, ty.clone())); } let typemap = TypeMap { From faedbd6de55d4864be09f5b4085a4055b2eaeb1d Mon Sep 17 00:00:00 2001 From: Cormac Relf Date: Mon, 17 Nov 2025 21:05:21 +1100 Subject: [PATCH 09/10] Delete dead code post recursive function type checker --- starlark-rust/starlark/src/typing/bindings.rs | 35 ++++--------------- 1 file changed, 7 insertions(+), 28 deletions(-) diff --git a/starlark-rust/starlark/src/typing/bindings.rs b/starlark-rust/starlark/src/typing/bindings.rs index bea1abe399f49..82c82b8493658 100644 --- a/starlark-rust/starlark/src/typing/bindings.rs +++ b/starlark-rust/starlark/src/typing/bindings.rs @@ -111,7 +111,7 @@ pub(crate) struct ChildDef<'a> { pub(crate) struct BindingsCollect<'a, 'b> { pub(crate) bindings: Bindings<'a>, pub(crate) approximations: &'b mut Vec, - pub(crate) children_sink: Option<&'b mut Vec>>, + pub(crate) children_sink: &'b mut Vec>, } impl<'a, 'b> BindingsCollect<'a, 'b> { @@ -130,7 +130,7 @@ impl<'a, 'b> BindingsCollect<'a, 'b> { let mut res = BindingsCollect { bindings: Bindings::default(), approximations, - children_sink: Some(children_sink), + children_sink, }; res.bindings.types = visible.clone(); @@ -138,25 +138,6 @@ impl<'a, 'b> BindingsCollect<'a, 'b> { Ok(res.bindings) } - /// Collect all the assignments to variables. - /// - /// This function only fails on internal errors. - pub(crate) fn collect_one( - x: &'a mut CstStmt, - typecheck_mode: TypecheckMode, - codemap: &CodeMap, - approximations: &'b mut Vec, - ) -> Result { - let mut res = BindingsCollect { - bindings: Bindings::default(), - approximations, - children_sink: None, - }; - - res.visit(Visit::Stmt(x), &Ty::any(), typecheck_mode, codemap)?; - Ok(res) - } - fn assign( &mut self, lhs: &'a CstAssignTarget, @@ -342,13 +323,11 @@ impl<'a, 'b> BindingsCollect<'a, 'b> { def.visit_header_err(|x| self.visit(x, &ret_ty, typecheck_mode, codemap))?; // Function body just gets added to the queue. - if let Some(sink) = &mut self.children_sink { - sink.push(ChildDef { - body: &def.body, - param_types, - return_type: ret_ty, - }); - } + self.children_sink.push(ChildDef { + body: &def.body, + param_types, + return_type: ret_ty, + }); Ok(()) } From 8e1a6df4b54002c3fbfc7dfb5cb454423c2a41d8 Mon Sep 17 00:00:00 2001 From: Cormac Relf Date: Wed, 12 Nov 2025 22:33:13 +1100 Subject: [PATCH 10/10] Fix TyUser not forwarding bin_op This makes the prelude type-check again after the recursive type checker was introduced. Errors were e.g 'RunInfo | None' not finding binop |. --- starlark-rust/starlark/src/typing/user.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/starlark-rust/starlark/src/typing/user.rs b/starlark-rust/starlark/src/typing/user.rs index c087a7432ce5c..b1b5928251cd7 100644 --- a/starlark-rust/starlark/src/typing/user.rs +++ b/starlark-rust/starlark/src/typing/user.rs @@ -292,6 +292,15 @@ impl TyCustomImpl for TyUser { } self.supertypes.iter().any(|x| x == other) } + + fn bin_op( + &self, + bin_op: super::TypingBinOp, + rhs: &TyBasic, + _ctx: &TypingOracleCtx, + ) -> Result { + Ok(self.base.bin_op(bin_op, rhs)?) + } } #[cfg(test)]