[red-knot] Attribute access and the descriptor protocol

[sharkdp]

Summary

• Attributes/method are now properly looked up on metaclasses, when called on class objects
• We properly distinguish between data descriptors and non-data descriptors (but we do not yet support them in store-context, i.e. obj.data_descr = …)
• The descriptor protocol is now implemented in a single unified place for instances, classes and dunder-calls. Unions and possibly-unbound symbols are supported in all possible stages of the process by creating union types as results.
• In general, the handling of "possibly-unbound" symbols has been improved in a lot of places: meta-class attributes, attributes, descriptors with possibly-unbound __get__ methods, instance attributes, …
• We keep track of type qualifiers in a lot more places. I anticipate that this will be useful if we import e.g. Final symbols from other modules (see relevant change to typing spec: Added "Importing Final Variables" section python/typing#1937).
• Detection and special-casing of the typing.Protocol special form in order to avoid lots of changes in the test suite due to new @Todo types when looking up attributes on builtin types which have Protocol in their MRO. We previously
  looked up attributes in a wrong way, which is why this didn't come up before.

closes #16367
closes #15966

Context

The way attribute lookup in Type::member worked before was simply wrong (mostly my own fault). The whole instance-attribute lookup should probably never have been integrated into Type::member. And the Type::static_member function that I introduced in my last descriptor PR was the wrong abstraction. It's kind of fascinating how far this approach took us, but I am pretty confident that the new approach proposed here is what we need to model this correctly.

There are three key pieces that are required to implement attribute lookups:

• Type::class_member/Type::find_in_mro: The Type::find_in_mro method that can look up attributes on class bodies (and corresponding bases). This is a partial function on types, as it can not be called on instance types likeType::Instance(…) or Type::IntLiteral(…). For this reason, we usually call it through Type::class_member, which is essentially just type.to_meta_type().find_in_mro(…) plus union/intersection handling.
• Type::instance_member: This new function is basically the type-level equivalent to obj.__dict__[name] when called on Type::Instance(…). We use this to discover instance attributes such as those that we see as declarations on class bodies or as (annotated) assignments to self.attr in methods of a class.
• The implementation of the descriptor protocol. It works slightly different for instances and for class objects, but it can be described by the general framework:
  • Call type.class_member("attribute") to look up "attribute" in the MRO of the meta type of type. Call the resulting Symbol meta_attr (even if it's unbound).
  • Use meta_attr.class_member("__get__") to look up __get__ on the meta type of meta_attr. Call it with __get__(meta_attr, self, self.to_meta_type()). If this fails (either the lookup or the call), just proceed with meta_attr. Otherwise, replace meta_attr in the following with the return type of __get__. In this step, we also probe if a __set__ or __delete__ method exists and store it in meta_attr_kind (can be either "data descriptor" or "normal attribute or non-data descriptor").
  • Compute a fallback type.
    • For instances, we use self.instance_member("attribute")
    • For class objects, we use class_attr = self.find_in_mro("attribute"), and then try to invoke the descriptor protocol on class_attr, i.e. we look up __get__ on the meta type of class_attr and call it with __get__(class_attr, None, self). This additional invocation of the descriptor protocol on the fallback type is one major asymmetry in the otherwise universal descriptor protocol implementation.
  • Finally, we look at meta_attr, meta_attr_kind and fallback, and handle various cases of (possible) unboundness of these symbols.
    • If meta_attr is bound and a data descriptor, just return meta_attr
    • If meta_attr is not a data descriptor, and fallback is bound, just return fallback
    • If meta_attr is not a data descriptor, and fallback is unbound, return meta_attr
    • Return unions of these three possibilities for partially-bound symbols.

This allows us to handle class objects and instances within the same framework. There is a minor additional detail where for instances, we do not allow the fallback type (the instance attribute) to completely shadow the non-data descriptor. We do this because we (currently) don't want to pretend that we can statically infer that an instance attribute is always set.

Dunder method calls can also be embedded into this framework. The only thing that changes is that there is no fallback type. If a dunder method is called on an instance, we do not fall back to instance variables. If a dunder method is called on a class object, we only look it up on the meta class, never on the class itself.

Test Plan

New Markdown tests.

[codspeed-hq]

CodSpeed Performance Report

Merging #16416 will degrade performances by 4.29%

_{Comparing david/descriptor-protocol-pt2 (689053e) with main (a18d8bf)}

Summary

❌ 1 (👁 1) regressions
✅ 31 untouched benchmarks

Benchmarks breakdown

	Benchmark	BASE	HEAD	Change
👁	red_knot_check_file[incremental]	5.2 ms	5.5 ms	-4.29%

[carljm]

This is excellent! Left some comments but IMO nothing that can't be left as a TODO and addressed as follow up; probably better to land this sooner rather than later.

[carljm]

I'm not sure about this case. If it's not explicitly marked as ClassVar, shouldn't we consider this a possible instance attribute, which might be found in __dict__? (We would allow assignment of it on an instance.)

[sharkdp]

So this is the case I talked about yesterday. It's definitely not straightforward to just return declared.with_qualifiers(qualifiers) here. If we do so, we treat every function on a class (which is declared and bound) into an instance attribute. Which does not just feel wrong, but also produces a lot of test failures. Instance attributes take precedence over non-data descriptors, so that would mean that we do not generate bound methods for something like C().some_method, but a normal Literal function type.

[sharkdp]

I decided to merge this PR before signing off for the weekend, but curious to hear your opinion here. I think what I implemented here (w.r.t. this case) mostly keeps the behavior as it was before, but that doesn't necessarily mean that it's the best possible behavior. It's certainly somewhat inconsistent with how we treat declared-but-unbound attributes on the class body (for which we also see no attribute assignments). Another way to make it consistent would be to make those also return Unbound (maybe except for stubs), unless we see at least one attribute binding in a method(?).

[carljm]

The inconsistency that I was thinking about here is that if we see e.g. foo: int = 1 in a class body (without a ClassVar annotation), we generally consider that to be an instance attribute with class fallback value. This means we will allow it to be set on an instance, which means it could exist in the instance dictionary, which means it could shadow a non-data descriptor. But I think you're right that we have to assume it does not, otherwise non-data descriptors would never work at all. It's not just methods, it's anytime you do d: NonDataDescriptor = NonDataDescriptor() on a class, we would think C().d might give you back NonDataDescriptor, because there might be one of those set in the instance dict. This is definitely not useful behavior.

So I think what you've done here is right! We will see if any issues come up on real codebases and can tweak accordingly.

[AlexWaygood]

Not a proper review as I'm on my phone, but a couple of minor things I spotted skimming through the diff. Nothing blocking!