Skip to content

[red-knot] Refactor: no mutability in call APIs #17788

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

Merged
sharkdp merged 1 commit into from
May 2, 2025
Merged

[red-knot] Refactor: no mutability in call APIs #17788

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
11 changes: 4 additions & 7 deletions crates/red_knot_python_semantic/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2649,10 +2649,7 @@ impl<'db> Type<'db> {

if let Symbol::Type(descr_get, descr_get_boundness) = descr_get {
let return_ty = descr_get
.try_call(
db,
&mut CallArgumentTypes::positional([self, instance, owner]),
)
.try_call(db, &CallArgumentTypes::positional([self, instance, owner]))
.map(|bindings| {
if descr_get_boundness == Boundness::Bound {
bindings.return_type(db)
Expand Down Expand Up @@ -4207,7 +4204,7 @@ impl<'db> Type<'db> {
fn try_call(
self,
db: &'db dyn Db,
argument_types: &mut CallArgumentTypes<'_, 'db>,
argument_types: &CallArgumentTypes<'_, 'db>,
) -> Result<Bindings<'db>, CallError<'db>> {
let signatures = self.signatures(db);
Bindings::match_parameters(signatures, argument_types).check_types(db, argument_types)
Expand Down Expand Up @@ -4420,7 +4417,7 @@ impl<'db> Type<'db> {
fn try_call_constructor(
self,
db: &'db dyn Db,
mut argument_types: CallArgumentTypes<'_, 'db>,
argument_types: CallArgumentTypes<'_, 'db>,
) -> Result<Type<'db>, ConstructorCallError<'db>> {
debug_assert!(matches!(
self,
Expand Down Expand Up @@ -4486,7 +4483,7 @@ impl<'db> Type<'db> {

match new_method {
Symbol::Type(new_method, boundness) => {
let result = new_method.try_call(db, argument_types);
let result = new_method.try_call(db, &argument_types);

if boundness == Boundness::PossiblyUnbound {
return Some(Err(DunderNewCallError::PossiblyUnbound(result.err())));
Expand Down
33 changes: 18 additions & 15 deletions crates/red_knot_python_semantic/src/types/call/arguments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,18 @@ impl<'a> CallArguments<'a> {
/// Invoke a function with an optional extra synthetic argument (for a `self` or `cls`
/// parameter) prepended to the front of this argument list. (If `bound_self` is none, the
/// function is invoked with the unmodified argument list.)
pub(crate) fn with_self<F, R>(&mut self, bound_self: Option<Type<'_>>, f: F) -> R
pub(crate) fn with_self<F, R>(&self, bound_self: Option<Type<'_>>, f: F) -> R
where
F: FnOnce(&mut Self) -> R,
F: FnOnce(&Self) -> R,
{
let mut call_arguments = self.clone();
Copy link
Member

@dcreager dcreager May 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to clone when there's no bound_self. Opened #17793 for that, and to combine the clone/push_front into a single collect call, which should avoid a double-copy when there is a bound_self. (Or really, a copy + immediate move)


if bound_self.is_some() {
self.0.push_front(Argument::Synthetic);
call_arguments.0.push_front(Argument::Synthetic);
}
let result = f(self);
let result = f(&call_arguments);
if bound_self.is_some() {
self.0.pop_front();
call_arguments.0.pop_front();
}
result
}
Expand Down Expand Up @@ -55,6 +57,7 @@ pub(crate) enum Argument<'a> {
}

/// Arguments for a single call, in source order, along with inferred types for each argument.
#[derive(Clone, Debug)]
pub(crate) struct CallArgumentTypes<'a, 'db> {
arguments: CallArguments<'a>,
types: VecDeque<Type<'db>>,
Expand Down Expand Up @@ -93,20 +96,20 @@ impl<'a, 'db> CallArgumentTypes<'a, 'db> {
/// Invoke a function with an optional extra synthetic argument (for a `self` or `cls`
/// parameter) prepended to the front of this argument list. (If `bound_self` is none, the
/// function is invoked with the unmodified argument list.)
pub(crate) fn with_self<F, R>(&mut self, bound_self: Option<Type<'db>>, f: F) -> R
pub(crate) fn with_self<F, R>(&self, bound_self: Option<Type<'db>>, f: F) -> R
where
F: FnOnce(&mut Self) -> R,
F: FnOnce(Self) -> R,
{
let mut call_argument_types = self.clone();

if let Some(bound_self) = bound_self {
self.arguments.0.push_front(Argument::Synthetic);
self.types.push_front(bound_self);
call_argument_types
.arguments
.0
.push_front(Argument::Synthetic);
call_argument_types.types.push_front(bound_self);
}
let result = f(self);
if bound_self.is_some() {
self.arguments.0.pop_front();
self.types.pop_front();
}
result
f(call_argument_types)
}

pub(crate) fn iter(&self) -> impl Iterator<Item = (Argument<'a>, Type<'db>)> + '_ {
Expand Down
24 changes: 9 additions & 15 deletions crates/red_knot_python_semantic/src/types/call/bind.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl<'db> Bindings<'db> {
/// verify that each argument type is assignable to the corresponding parameter type.
pub(crate) fn match_parameters(
signatures: Signatures<'db>,
arguments: &mut CallArguments<'_>,
arguments: &CallArguments<'_>,
) -> Self {
let mut argument_forms = vec![None; arguments.len()];
let mut conflicting_forms = vec![false; arguments.len()];
Expand Down Expand Up @@ -92,7 +92,7 @@ impl<'db> Bindings<'db> {
pub(crate) fn check_types(
mut self,
db: &'db dyn Db,
argument_types: &mut CallArgumentTypes<'_, 'db>,
argument_types: &CallArgumentTypes<'_, 'db>,
) -> Result<Self, CallError<'db>> {
for (signature, element) in self.signatures.iter().zip(&mut self.elements) {
element.check_types(db, signature, argument_types);
Expand Down Expand Up @@ -351,10 +351,7 @@ impl<'db> Bindings<'db> {
[Some(Type::PropertyInstance(property)), Some(instance), ..] => {
if let Some(getter) = property.getter(db) {
if let Ok(return_ty) = getter
.try_call(
db,
&mut CallArgumentTypes::positional([*instance]),
)
.try_call(db, &CallArgumentTypes::positional([*instance]))
.map(|binding| binding.return_type(db))
{
overload.set_return_type(return_ty);
Expand Down Expand Up @@ -383,10 +380,7 @@ impl<'db> Bindings<'db> {
[Some(instance), ..] => {
if let Some(getter) = property.getter(db) {
if let Ok(return_ty) = getter
.try_call(
db,
&mut CallArgumentTypes::positional([*instance]),
)
.try_call(db, &CallArgumentTypes::positional([*instance]))
.map(|binding| binding.return_type(db))
{
overload.set_return_type(return_ty);
Expand Down Expand Up @@ -414,7 +408,7 @@ impl<'db> Bindings<'db> {
if let Some(setter) = property.setter(db) {
if let Err(_call_error) = setter.try_call(
db,
&mut CallArgumentTypes::positional([*instance, *value]),
&CallArgumentTypes::positional([*instance, *value]),
) {
overload.errors.push(BindingError::InternalCallError(
"calling the setter failed",
Expand All @@ -433,7 +427,7 @@ impl<'db> Bindings<'db> {
if let Some(setter) = property.setter(db) {
if let Err(_call_error) = setter.try_call(
db,
&mut CallArgumentTypes::positional([*instance, *value]),
&CallArgumentTypes::positional([*instance, *value]),
) {
overload.errors.push(BindingError::InternalCallError(
"calling the setter failed",
Expand Down Expand Up @@ -874,7 +868,7 @@ pub(crate) struct CallableBinding<'db> {
impl<'db> CallableBinding<'db> {
fn match_parameters(
signature: &CallableSignature<'db>,
arguments: &mut CallArguments<'_>,
arguments: &CallArguments<'_>,
argument_forms: &mut [Option<ParameterForm>],
conflicting_forms: &mut [bool],
) -> Self {
Expand Down Expand Up @@ -912,13 +906,13 @@ impl<'db> CallableBinding<'db> {
&mut self,
db: &'db dyn Db,
signature: &CallableSignature<'db>,
argument_types: &mut CallArgumentTypes<'_, 'db>,
argument_types: &CallArgumentTypes<'_, 'db>,
) {
// If this callable is a bound method, prepend the self instance onto the arguments list
// before checking.
argument_types.with_self(signature.bound_type, |argument_types| {
for (signature, overload) in signature.iter().zip(&mut self.overloads) {
overload.check_types(db, signature, argument_types);
overload.check_types(db, signature, &argument_types);
}
});
}
Expand Down
4 changes: 2 additions & 2 deletions crates/red_knot_python_semantic/src/types/class.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -772,9 +772,9 @@ impl<'db> ClassLiteral<'db> {
let namespace = KnownClass::Dict.to_instance(db);

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

let return_ty_result = match metaclass.try_call(db, &mut arguments) {
let return_ty_result = match metaclass.try_call(db, &arguments) {
Ok(bindings) => Ok(bindings.return_type(db)),

Err(CallError(CallErrorKind::NotCallable, bindings)) => Err(MetaclassError {
Expand Down
24 changes: 12 additions & 12 deletions crates/red_knot_python_semantic/src/types/infer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1815,7 +1815,7 @@ impl<'db> TypeInferenceBuilder<'db> {

for (decorator_ty, decorator_node) in decorator_types_and_nodes.iter().rev() {
inferred_ty = match decorator_ty
.try_call(self.db(), &mut CallArgumentTypes::positional([inferred_ty]))
.try_call(self.db(), &CallArgumentTypes::positional([inferred_ty]))
.map(|bindings| bindings.return_type(self.db()))
{
Ok(return_ty) => return_ty,
Expand Down Expand Up @@ -2832,7 +2832,7 @@ impl<'db> TypeInferenceBuilder<'db> {
let successful_call = meta_dunder_set
.try_call(
db,
&mut CallArgumentTypes::positional([
&CallArgumentTypes::positional([
meta_attr_ty,
object_ty,
value_ty,
Expand Down Expand Up @@ -2973,7 +2973,7 @@ impl<'db> TypeInferenceBuilder<'db> {
let successful_call = meta_dunder_set
.try_call(
db,
&mut CallArgumentTypes::positional([
&CallArgumentTypes::positional([
meta_attr_ty,
object_ty,
value_ty,
Expand Down Expand Up @@ -4561,7 +4561,7 @@ impl<'db> TypeInferenceBuilder<'db> {
// We don't call `Type::try_call`, because we want to perform type inference on the
// arguments after matching them to parameters, but before checking that the argument types
// are assignable to any parameter annotations.
let mut call_arguments = Self::parse_arguments(arguments);
let call_arguments = Self::parse_arguments(arguments);
let callable_type = self.infer_expression(func);

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

let signatures = callable_type.signatures(self.db());
let bindings = Bindings::match_parameters(signatures, &mut call_arguments);
let mut call_argument_types =
let bindings = Bindings::match_parameters(signatures, &call_arguments);
let call_argument_types =
self.infer_argument_types(arguments, call_arguments, &bindings.argument_forms);

match bindings.check_types(self.db(), &mut call_argument_types) {
match bindings.check_types(self.db(), &call_argument_types) {
Ok(mut bindings) => {
for binding in &mut bindings {
let binding_type = binding.callable_type;
Expand Down Expand Up @@ -6486,7 +6486,7 @@ impl<'db> TypeInferenceBuilder<'db> {
Symbol::Type(contains_dunder, Boundness::Bound) => {
// If `__contains__` is available, it is used directly for the membership test.
contains_dunder
.try_call(db, &mut CallArgumentTypes::positional([right, left]))
.try_call(db, &CallArgumentTypes::positional([right, left]))
.map(|bindings| bindings.return_type(db))
.ok()
}
Expand Down Expand Up @@ -6640,7 +6640,7 @@ impl<'db> TypeInferenceBuilder<'db> {
generic_context: GenericContext<'db>,
) -> Type<'db> {
let slice_node = subscript.slice.as_ref();
let mut call_argument_types = match slice_node {
let call_argument_types = match slice_node {
ast::Expr::Tuple(tuple) => CallArgumentTypes::positional(
tuple.elts.iter().map(|elt| self.infer_type_expression(elt)),
),
Expand All @@ -6650,8 +6650,8 @@ impl<'db> TypeInferenceBuilder<'db> {
value_ty,
generic_context.signature(self.db()),
));
let bindings = match Bindings::match_parameters(signatures, &mut call_argument_types)
.check_types(self.db(), &mut call_argument_types)
let bindings = match Bindings::match_parameters(signatures, &call_argument_types)
.check_types(self.db(), &call_argument_types)
{
Ok(bindings) => bindings,
Err(CallError(_, bindings)) => {
Expand Down Expand Up @@ -6893,7 +6893,7 @@ impl<'db> TypeInferenceBuilder<'db> {

match ty.try_call(
self.db(),
&mut CallArgumentTypes::positional([value_ty, slice_ty]),
&CallArgumentTypes::positional([value_ty, slice_ty]),
) {
Ok(bindings) => return bindings.return_type(self.db()),
Err(CallError(_, bindings)) => {
Expand Down
Loading