Skip to content

Commit 9937064

Browse files
authored
[red-knot] Use iterative approach to collect overloads (#17607)
## Summary This PR updates the `to_overloaded` method to use an iterative approach instead of a recursive one. Refer to #17585 (comment) for context. The main benefit here is that it avoids calling the `to_overloaded` function in a recursive manner which is a salsa query. So, this is a bit hand wavy but we should also see less memory used because the cache will only contain a single entry which should be the entire overload chain. Previously, the recursive approach would mean that each of the function involved in an overload chain would have a cache entry. This reduce in memory shouldn't be too much and I haven't looked at the actual data for it. ## Test Plan Existing test cases should pass.
1 parent 8d2c792 commit 9937064

File tree

1 file changed

+43
-53
lines changed
  • crates/red_knot_python_semantic/src

1 file changed

+43
-53
lines changed

crates/red_knot_python_semantic/src/types.rs

+43-53
Original file line numberDiff line numberDiff line change
@@ -6308,64 +6308,54 @@ impl<'db> FunctionType<'db> {
63086308
db: &'db dyn Db,
63096309
function: FunctionType<'db>,
63106310
) -> Option<OverloadedFunction<'db>> {
6311-
// The semantic model records a use for each function on the name node. This is used here
6312-
// to get the previous function definition with the same name.
6313-
let scope = function.definition(db).scope(db);
6314-
let use_def = semantic_index(db, scope.file(db)).use_def_map(scope.file_scope_id(db));
6315-
let use_id = function
6316-
.body_scope(db)
6317-
.node(db)
6318-
.expect_function()
6319-
.name
6320-
.scoped_use_id(db, scope);
6321-
6322-
if let Symbol::Type(Type::FunctionLiteral(function_literal), Boundness::Bound) =
6323-
symbol_from_bindings(db, use_def.bindings_at_use(use_id))
6324-
{
6325-
match function_literal.to_overloaded(db) {
6326-
None => {
6327-
debug_assert!(
6328-
!function_literal.has_known_decorator(db, FunctionDecorators::OVERLOAD),
6329-
"Expected `Some(OverloadedFunction)` if the previous function was an overload"
6330-
);
6331-
}
6332-
Some(OverloadedFunction {
6333-
implementation: Some(_),
6334-
..
6335-
}) => {
6336-
// If the previous overloaded function already has an implementation, then this
6337-
// new signature completely replaces it.
6338-
}
6339-
Some(OverloadedFunction {
6340-
overloads,
6341-
implementation: None,
6342-
}) => {
6343-
return Some(
6344-
if function.has_known_decorator(db, FunctionDecorators::OVERLOAD) {
6345-
let mut overloads = overloads.clone();
6346-
overloads.push(function);
6347-
OverloadedFunction {
6348-
overloads,
6349-
implementation: None,
6350-
}
6351-
} else {
6352-
OverloadedFunction {
6353-
overloads: overloads.clone(),
6354-
implementation: Some(function),
6355-
}
6356-
},
6357-
);
6358-
}
6311+
let mut current = function;
6312+
let mut overloads = vec![];
6313+
6314+
loop {
6315+
// The semantic model records a use for each function on the name node. This is used
6316+
// here to get the previous function definition with the same name.
6317+
let scope = current.definition(db).scope(db);
6318+
let use_def =
6319+
semantic_index(db, scope.file(db)).use_def_map(scope.file_scope_id(db));
6320+
let use_id = current
6321+
.body_scope(db)
6322+
.node(db)
6323+
.expect_function()
6324+
.name
6325+
.scoped_use_id(db, scope);
6326+
6327+
let Symbol::Type(Type::FunctionLiteral(previous), Boundness::Bound) =
6328+
symbol_from_bindings(db, use_def.bindings_at_use(use_id))
6329+
else {
6330+
break;
6331+
};
6332+
6333+
if previous.has_known_decorator(db, FunctionDecorators::OVERLOAD) {
6334+
overloads.push(previous);
6335+
} else {
6336+
break;
63596337
}
6338+
6339+
current = previous;
63606340
}
63616341

6362-
if function.has_known_decorator(db, FunctionDecorators::OVERLOAD) {
6363-
Some(OverloadedFunction {
6364-
overloads: vec![function],
6365-
implementation: None,
6366-
})
6342+
// Overloads are inserted in reverse order, from bottom to top.
6343+
overloads.reverse();
6344+
6345+
let implementation = if function.has_known_decorator(db, FunctionDecorators::OVERLOAD) {
6346+
overloads.push(function);
6347+
None
63676348
} else {
6349+
Some(function)
6350+
};
6351+
6352+
if overloads.is_empty() {
63686353
None
6354+
} else {
6355+
Some(OverloadedFunction {
6356+
overloads,
6357+
implementation,
6358+
})
63696359
}
63706360
}
63716361

0 commit comments

Comments
 (0)