use the ClassDef attribute inference process in Instances #2563
Conversation
|
Note that the biggest source of changes is just moving a bunch of code into a separate function |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2563 +/- ##
==========================================
+ Coverage 93.05% 93.09% +0.03%
==========================================
Files 93 93
Lines 11050 11053 +3
==========================================
+ Hits 10283 10290 +7
+ Misses 767 763 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Instances often have «special_attributes». ClassDef does a lot of transformations of attibutes during inference, so let's reuse them in Instance. We are fixing inference of the "special" attributes of Instances. Before these changes, the FunctionDef attributes wouldn't get wrapped into a BoundMethod. This was facilitated by extracting the logic of inferring attributes into 'FunctionDef._infer_attrs' and reusing it in BaseInstance. This issue isn't visible right now, because the special attributes are simply not found due not being attached to the parent (the instance). Which in turn is caused by not providing the actual parent in the constructor. Since we are on a quest to always provide a parent, this change is necessary.
temyurchenko
force-pushed
the
classdef-inference-in-instance
branch
from
ce64792
to
f2c3947
Compare
| @@ -1295,7 +1295,7 @@ def func(arg1, arg2): | |||
| self.assertIsInstance(inferred[0], BoundMethod) | |||
| inferred = list(Instance(cls).igetattr("m4")) | |||
| self.assertEqual(len(inferred), 1) | |||
| self.assertIsInstance(inferred[0], nodes.FunctionDef) | |||
| self.assertIsInstance(inferred[0], BoundMethod) | |||
There was a problem hiding this comment.
I think this change is incorrect, and therefore shows something is up with this PR.
To reproduce:
class Clazz(object): ...
def func(arg1, arg2):
"function that will be used as a method"
return arg1.value + arg2
Clazz.m3 = func
inst = Clazz()
inst.m4 = func
print(Clazz.m3)
print(inst.m3)
print(inst.m4)<function func at 0x109582340>
<bound method func of <__main__.Clazz object at 0x1095ab9b0>>
<function func at 0x109582340>This should remain a FunctionDef and not a BoundMethod.
There was a problem hiding this comment.
Hmm, you are right. Then, I think the issue is in the semantics of objectmodel. For ordinary Instances, special_attributes are «instance» attributes. They should remain FunctionDef. However, in some cases, for specialized instances, e.g. Generator, the objectmodel is used for «class» methods. So, the FunctionDefs should get wrapped into BoundMethods.
So, the fix should lie on that side, but it also feels quite error-prone to use the same special_attributes for the two different purposes.
I'll do a fix, but a bit later, I'm a bit short on time.
|
Closing in favour of #2584 |
A part of getting rid of non-Module roots, see #2536
Instances often have «special_attributes». ClassDef does a lot of
transformations of attibutes during inference, so let's reuse them
in Instance.
We are fixing inference of the "special" attributes of
Instances. Before these changes, the FunctionDef attributes
wouldn't get wrapped into a BoundMethod. This was facilitated by
extracting the logic of inferring attributes into
'FunctionDef._infer_attrs' and reusing it in BaseInstance.
This issue isn't visible right now, because the special attributes
are simply not found due not being attached to the parent (the
instance). Which in turn is caused by not providing the actual
parent in the constructor. Since we are on a quest to always
provide a parent, this change is necessary.