Skip to content

Commit ea3f4ac

Browse files
sharkdpsharkdp
authored
[red-knot] Refactor: no mutability in call APIs (#17788)
## Summary Remove mutability in parameter types for a few functions such as `with_self` and `try_call`. I tried the `Rc`-approach with cheap cloning [suggest here](#17733 (comment)) first, but it turns out we need a whole stack of prepended arguments (there can be [both `self` *and* `cls`](https://github.com/astral-sh/ruff/blob/3cf44e401a64658c17652cd3a17c897dc50261eb/crates/red_knot_python_semantic/resources/mdtest/call/constructor.md?plain=1#L113)), and we would need the same construct not just for `CallArguments` but also for `CallArgumentTypes`. At that point we're cloning `VecDeque`s anyway, so the overhead of cloning the whole `VecDeque` with all arguments didn't seem to justify the additional code complexity. ## Benchmarks Benchmarks on tomllib, black, jinja, isort seem neutral.
1 parent 6d2c10c commit ea3f4ac

File tree

5 files changed

+45
-51
lines changed

5 files changed

+45
-51
lines changed

‎crates/red_knot_python_semantic/src/types.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2649,10 +2649,7 @@ impl<'db> Type<'db> {
26492649

26502650
if let Symbol::Type(descr_get, descr_get_boundness) = descr_get {
26512651
let return_ty = descr_get
2652-
.try_call(
2653-
db,
2654-
&mut CallArgumentTypes::positional([self, instance, owner]),
2655-
)
2652+
.try_call(db, &CallArgumentTypes::positional([self, instance, owner]))
26562653
.map(|bindings| {
26572654
if descr_get_boundness == Boundness::Bound {
26582655
bindings.return_type(db)
@@ -4207,7 +4204,7 @@ impl<'db> Type<'db> {
42074204
fn try_call(
42084205
self,
42094206
db: &'db dyn Db,
4210-
argument_types: &mut CallArgumentTypes<'_, 'db>,
4207+
argument_types: &CallArgumentTypes<'_, 'db>,
42114208
) -> Result<Bindings<'db>, CallError<'db>> {
42124209
let signatures = self.signatures(db);
42134210
Bindings::match_parameters(signatures, argument_types).check_types(db, argument_types)
@@ -4420,7 +4417,7 @@ impl<'db> Type<'db> {
44204417
fn try_call_constructor(
44214418
self,
44224419
db: &'db dyn Db,
4423-
mut argument_types: CallArgumentTypes<'_, 'db>,
4420+
argument_types: CallArgumentTypes<'_, 'db>,
44244421
) -> Result<Type<'db>, ConstructorCallError<'db>> {
44254422
debug_assert!(matches!(
44264423
self,
@@ -4486,7 +4483,7 @@ impl<'db> Type<'db> {
44864483

44874484
match new_method {
44884485
Symbol::Type(new_method, boundness) => {
4489-
let result = new_method.try_call(db, argument_types);
4486+
let result = new_method.try_call(db, &argument_types);
44904487

44914488
if boundness == Boundness::PossiblyUnbound {
44924489
return Some(Err(DunderNewCallError::PossiblyUnbound(result.err())));

‎crates/red_knot_python_semantic/src/types/call/arguments.rs

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,18 @@ impl<'a> CallArguments<'a> {
1111
/// Invoke a function with an optional extra synthetic argument (for a `self` or `cls`
1212
/// parameter) prepended to the front of this argument list. (If `bound_self` is none, the
1313
/// function is invoked with the unmodified argument list.)
14-
pub(crate) fn with_self<F, R>(&mut self, bound_self: Option<Type<'_>>, f: F) -> R
14+
pub(crate) fn with_self<F, R>(&self, bound_self: Option<Type<'_>>, f: F) -> R
1515
where
16-
F: FnOnce(&mut Self) -> R,
16+
F: FnOnce(&Self) -> R,
1717
{
18+
let mut call_arguments = self.clone();
19+
1820
if bound_self.is_some() {
19-
self.0.push_front(Argument::Synthetic);
21+
call_arguments.0.push_front(Argument::Synthetic);
2022
}
21-
let result = f(self);
23+
let result = f(&call_arguments);
2224
if bound_self.is_some() {
23-
self.0.pop_front();
25+
call_arguments.0.pop_front();
2426
}
2527
result
2628
}
@@ -55,6 +57,7 @@ pub(crate) enum Argument<'a> {
5557
}
5658

5759
/// Arguments for a single call, in source order, along with inferred types for each argument.
60+
#[derive(Clone, Debug)]
5861
pub(crate) struct CallArgumentTypes<'a, 'db> {
5962
arguments: CallArguments<'a>,
6063
types: VecDeque<Type<'db>>,
@@ -93,20 +96,20 @@ impl<'a, 'db> CallArgumentTypes<'a, 'db> {
9396
/// Invoke a function with an optional extra synthetic argument (for a `self` or `cls`
9497
/// parameter) prepended to the front of this argument list. (If `bound_self` is none, the
9598
/// function is invoked with the unmodified argument list.)
96-
pub(crate) fn with_self<F, R>(&mut self, bound_self: Option<Type<'db>>, f: F) -> R
99+
pub(crate) fn with_self<F, R>(&self, bound_self: Option<Type<'db>>, f: F) -> R
97100
where
98-
F: FnOnce(&mut Self) -> R,
101+
F: FnOnce(Self) -> R,
99102
{
103+
let mut call_argument_types = self.clone();
104+
100105
if let Some(bound_self) = bound_self {
101-
self.arguments.0.push_front(Argument::Synthetic);
102-
self.types.push_front(bound_self);
106+
call_argument_types
107+
.arguments
108+
.0
109+
.push_front(Argument::Synthetic);
110+
call_argument_types.types.push_front(bound_self);
103111
}
104-
let result = f(self);
105-
if bound_self.is_some() {
106-
self.arguments.0.pop_front();
107-
self.types.pop_front();
108-
}
109-
result
112+
f(call_argument_types)
110113
}
111114

112115
pub(crate) fn iter(&self) -> impl Iterator<Item = (Argument<'a>, Type<'db>)> + '_ {

‎crates/red_knot_python_semantic/src/types/call/bind.rs

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ impl<'db> Bindings<'db> {
5656
/// verify that each argument type is assignable to the corresponding parameter type.
5757
pub(crate) fn match_parameters(
5858
signatures: Signatures<'db>,
59-
arguments: &mut CallArguments<'_>,
59+
arguments: &CallArguments<'_>,
6060
) -> Self {
6161
let mut argument_forms = vec![None; arguments.len()];
6262
let mut conflicting_forms = vec![false; arguments.len()];
@@ -92,7 +92,7 @@ impl<'db> Bindings<'db> {
9292
pub(crate) fn check_types(
9393
mut self,
9494
db: &'db dyn Db,
95-
argument_types: &mut CallArgumentTypes<'_, 'db>,
95+
argument_types: &CallArgumentTypes<'_, 'db>,
9696
) -> Result<Self, CallError<'db>> {
9797
for (signature, element) in self.signatures.iter().zip(&mut self.elements) {
9898
element.check_types(db, signature, argument_types);
@@ -351,10 +351,7 @@ impl<'db> Bindings<'db> {
351351
[Some(Type::PropertyInstance(property)), Some(instance), ..] => {
352352
if let Some(getter) = property.getter(db) {
353353
if let Ok(return_ty) = getter
354-
.try_call(
355-
db,
356-
&mut CallArgumentTypes::positional([*instance]),
357-
)
354+
.try_call(db, &CallArgumentTypes::positional([*instance]))
358355
.map(|binding| binding.return_type(db))
359356
{
360357
overload.set_return_type(return_ty);
@@ -383,10 +380,7 @@ impl<'db> Bindings<'db> {
383380
[Some(instance), ..] => {
384381
if let Some(getter) = property.getter(db) {
385382
if let Ok(return_ty) = getter
386-
.try_call(
387-
db,
388-
&mut CallArgumentTypes::positional([*instance]),
389-
)
383+
.try_call(db, &CallArgumentTypes::positional([*instance]))
390384
.map(|binding| binding.return_type(db))
391385
{
392386
overload.set_return_type(return_ty);
@@ -414,7 +408,7 @@ impl<'db> Bindings<'db> {
414408
if let Some(setter) = property.setter(db) {
415409
if let Err(_call_error) = setter.try_call(
416410
db,
417-
&mut CallArgumentTypes::positional([*instance, *value]),
411+
&CallArgumentTypes::positional([*instance, *value]),
418412
) {
419413
overload.errors.push(BindingError::InternalCallError(
420414
"calling the setter failed",
@@ -433,7 +427,7 @@ impl<'db> Bindings<'db> {
433427
if let Some(setter) = property.setter(db) {
434428
if let Err(_call_error) = setter.try_call(
435429
db,
436-
&mut CallArgumentTypes::positional([*instance, *value]),
430+
&CallArgumentTypes::positional([*instance, *value]),
437431
) {
438432
overload.errors.push(BindingError::InternalCallError(
439433
"calling the setter failed",
@@ -874,7 +868,7 @@ pub(crate) struct CallableBinding<'db> {
874868
impl<'db> CallableBinding<'db> {
875869
fn match_parameters(
876870
signature: &CallableSignature<'db>,
877-
arguments: &mut CallArguments<'_>,
871+
arguments: &CallArguments<'_>,
878872
argument_forms: &mut [Option<ParameterForm>],
879873
conflicting_forms: &mut [bool],
880874
) -> Self {
@@ -912,13 +906,13 @@ impl<'db> CallableBinding<'db> {
912906
&mut self,
913907
db: &'db dyn Db,
914908
signature: &CallableSignature<'db>,
915-
argument_types: &mut CallArgumentTypes<'_, 'db>,
909+
argument_types: &CallArgumentTypes<'_, 'db>,
916910
) {
917911
// If this callable is a bound method, prepend the self instance onto the arguments list
918912
// before checking.
919913
argument_types.with_self(signature.bound_type, |argument_types| {
920914
for (signature, overload) in signature.iter().zip(&mut self.overloads) {
921-
overload.check_types(db, signature, argument_types);
915+
overload.check_types(db, signature, &argument_types);
922916
}
923917
});
924918
}

‎crates/red_knot_python_semantic/src/types/class.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -772,9 +772,9 @@ impl<'db> ClassLiteral<'db> {
772772
let namespace = KnownClass::Dict.to_instance(db);
773773

774774
// TODO: Other keyword arguments?
775-
let mut arguments = CallArgumentTypes::positional([name, bases, namespace]);
775+
let arguments = CallArgumentTypes::positional([name, bases, namespace]);
776776

777-
let return_ty_result = match metaclass.try_call(db, &mut arguments) {
777+
let return_ty_result = match metaclass.try_call(db, &arguments) {
778778
Ok(bindings) => Ok(bindings.return_type(db)),
779779

780780
Err(CallError(CallErrorKind::NotCallable, bindings)) => Err(MetaclassError {

‎crates/red_knot_python_semantic/src/types/infer.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1815,7 +1815,7 @@ impl<'db> TypeInferenceBuilder<'db> {
18151815

18161816
for (decorator_ty, decorator_node) in decorator_types_and_nodes.iter().rev() {
18171817
inferred_ty = match decorator_ty
1818-
.try_call(self.db(), &mut CallArgumentTypes::positional([inferred_ty]))
1818+
.try_call(self.db(), &CallArgumentTypes::positional([inferred_ty]))
18191819
.map(|bindings| bindings.return_type(self.db()))
18201820
{
18211821
Ok(return_ty) => return_ty,
@@ -2832,7 +2832,7 @@ impl<'db> TypeInferenceBuilder<'db> {
28322832
let successful_call = meta_dunder_set
28332833
.try_call(
28342834
db,
2835-
&mut CallArgumentTypes::positional([
2835+
&CallArgumentTypes::positional([
28362836
meta_attr_ty,
28372837
object_ty,
28382838
value_ty,
@@ -2973,7 +2973,7 @@ impl<'db> TypeInferenceBuilder<'db> {
29732973
let successful_call = meta_dunder_set
29742974
.try_call(
29752975
db,
2976-
&mut CallArgumentTypes::positional([
2976+
&CallArgumentTypes::positional([
29772977
meta_attr_ty,
29782978
object_ty,
29792979
value_ty,
@@ -4561,7 +4561,7 @@ impl<'db> TypeInferenceBuilder<'db> {
45614561
// We don't call `Type::try_call`, because we want to perform type inference on the
45624562
// arguments after matching them to parameters, but before checking that the argument types
45634563
// are assignable to any parameter annotations.
4564-
let mut call_arguments = Self::parse_arguments(arguments);
4564+
let call_arguments = Self::parse_arguments(arguments);
45654565
let callable_type = self.infer_expression(func);
45664566

45674567
if let Type::FunctionLiteral(function) = callable_type {
@@ -4640,11 +4640,11 @@ impl<'db> TypeInferenceBuilder<'db> {
46404640
}
46414641

46424642
let signatures = callable_type.signatures(self.db());
4643-
let bindings = Bindings::match_parameters(signatures, &mut call_arguments);
4644-
let mut call_argument_types =
4643+
let bindings = Bindings::match_parameters(signatures, &call_arguments);
4644+
let call_argument_types =
46454645
self.infer_argument_types(arguments, call_arguments, &bindings.argument_forms);
46464646

4647-
match bindings.check_types(self.db(), &mut call_argument_types) {
4647+
match bindings.check_types(self.db(), &call_argument_types) {
46484648
Ok(mut bindings) => {
46494649
for binding in &mut bindings {
46504650
let binding_type = binding.callable_type;
@@ -6486,7 +6486,7 @@ impl<'db> TypeInferenceBuilder<'db> {
64866486
Symbol::Type(contains_dunder, Boundness::Bound) => {
64876487
// If `__contains__` is available, it is used directly for the membership test.
64886488
contains_dunder
6489-
.try_call(db, &mut CallArgumentTypes::positional([right, left]))
6489+
.try_call(db, &CallArgumentTypes::positional([right, left]))
64906490
.map(|bindings| bindings.return_type(db))
64916491
.ok()
64926492
}
@@ -6640,7 +6640,7 @@ impl<'db> TypeInferenceBuilder<'db> {
66406640
generic_context: GenericContext<'db>,
66416641
) -> Type<'db> {
66426642
let slice_node = subscript.slice.as_ref();
6643-
let mut call_argument_types = match slice_node {
6643+
let call_argument_types = match slice_node {
66446644
ast::Expr::Tuple(tuple) => CallArgumentTypes::positional(
66456645
tuple.elts.iter().map(|elt| self.infer_type_expression(elt)),
66466646
),
@@ -6650,8 +6650,8 @@ impl<'db> TypeInferenceBuilder<'db> {
66506650
value_ty,
66516651
generic_context.signature(self.db()),
66526652
));
6653-
let bindings = match Bindings::match_parameters(signatures, &mut call_argument_types)
6654-
.check_types(self.db(), &mut call_argument_types)
6653+
let bindings = match Bindings::match_parameters(signatures, &call_argument_types)
6654+
.check_types(self.db(), &call_argument_types)
66556655
{
66566656
Ok(bindings) => bindings,
66576657
Err(CallError(_, bindings)) => {
@@ -6893,7 +6893,7 @@ impl<'db> TypeInferenceBuilder<'db> {
68936893

68946894
match ty.try_call(
68956895
self.db(),
6896-
&mut CallArgumentTypes::positional([value_ty, slice_ty]),
6896+
&CallArgumentTypes::positional([value_ty, slice_ty]),
68976897
) {
68986898
Ok(bindings) => return bindings.return_type(self.db()),
68996899
Err(CallError(_, bindings)) => {

0 commit comments

Comments
 (0)