Skip to content

Commit 13bd099

Browse files
committed
More fixes for type identities.
It turns out that we were walking into arbitrary python objects. Our compiler doesn't, in fact, do that. For instance if you have class C: def __init__(self, x): self.x = x c = C(10) @entrypoint def f(): return c.x the compiler knows that 'x' might change and will not inline the constant. Unfortunately, our identityHasher was walking in and including the '10' in the value of the hash, which can lead to serious instability.
1 parent ef1be90 commit 13bd099

File tree

4 files changed

+53
-22
lines changed

4 files changed

+53
-22
lines changed

typed_python/CompilerVisibleObjectVisitor.hpp

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -703,20 +703,19 @@ class CompilerVisibleObjectVisitor {
703703
return;
704704
}
705705

706-
// we do want to visit the internals of arbitrary objects, because
707-
// the compiler will attempt to do so as well.
708-
if (PyObject_HasAttrString(obj.pyobj(), "__dict__")) {
709-
PyObjectStealer dict(PyObject_GetAttrString(obj.pyobj(), "__dict__"));
710-
711-
if (dict) {
712-
hashVisit(ShaHash(12));
713-
714-
instanceVisit((PyObject*)obj.pyobj()->ob_type);
715-
visitDict(dict, true);
716-
return;
717-
}
706+
if (obj.pyobj()->ob_type == &PyClassMethodDescr_Type
707+
|| obj.pyobj()->ob_type == &PyMethodDescr_Type) {
708+
// the compiler looks at the type and the name of a given method descriptor
709+
instanceVisit(PyDescr_TYPE(obj.pyobj()));
710+
instanceVisit(PyDescr_NAME(obj.pyobj()));
711+
return;
718712
}
719713

714+
// we don't visit the internals of arbitrary objects - by default, the compiler
715+
// won't do this because they are mutable.
716+
717+
// we do visit the type, since the compiler may infer something about the type
718+
// of the instance and we assume that type objects are stable.
720719
instanceVisit((PyObject*)obj.pyobj()->ob_type);
721720
}
722721

typed_python/MutuallyRecursiveTypeGroup.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -978,11 +978,8 @@ ShaHash MutuallyRecursiveTypeGroup::computePyObjectShaHashConstant(PyObject* h)
978978
}
979979

980980
if (h->ob_type == &PyProperty_Type
981-
|| h->ob_type == &PyProperty_Type
982-
|| h->ob_type == &PyClassMethodDescr_Type
983981
|| h->ob_type == &PyGetSetDescr_Type
984982
|| h->ob_type == &PyMemberDescr_Type
985-
|| h->ob_type == &PyMethodDescr_Type
986983
|| h->ob_type == &PyWrapperDescr_Type
987984
|| h->ob_type == &PyDictProxy_Type
988985
|| h->ob_type == &_PyMethodWrapper_Type
@@ -1032,11 +1029,8 @@ bool MutuallyRecursiveTypeGroup::isSimpleConstant(PyObject* h) {
10321029
|| h == (PyObject*)&PyFunction_Type
10331030
|| PyFloat_Check(h)
10341031
|| h->ob_type == &PyProperty_Type
1035-
|| h->ob_type == &PyProperty_Type
1036-
|| h->ob_type == &PyClassMethodDescr_Type
10371032
|| h->ob_type == &PyGetSetDescr_Type
10381033
|| h->ob_type == &PyMemberDescr_Type
1039-
|| h->ob_type == &PyMethodDescr_Type
10401034
|| h->ob_type == &PyWrapperDescr_Type
10411035
|| h->ob_type == &PyDictProxy_Type
10421036
|| h->ob_type == &_PyMethodWrapper_Type

typed_python/TypeOrPyobj.hpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,16 @@ class TypeOrPyobj {
4747
incref(mPyObj);
4848
}
4949

50+
TypeOrPyobj(PyTypeObject* o) :
51+
mType(nullptr),
52+
mPyObj((PyObject*)o)
53+
{
54+
if (!mPyObj) {
55+
throw std::runtime_error("Can't construct a TypeOrPyobj with a null PyObject");
56+
}
57+
incref(mPyObj);
58+
}
59+
5060
~TypeOrPyobj() {
5161
if (mPyObj) {
5262
decref(mPyObj);
@@ -162,4 +172,4 @@ namespace std {
162172
return hash<void*>()((void*)k.pyobj());
163173
}
164174
};
165-
}
175+
}

typed_python/type_identity_test.py

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,18 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
from typed_python.test_util import evaluateExprInFreshProcess
15+
import threading
1616
import pytest
17+
18+
import typed_python
19+
from typed_python.test_util import evaluateExprInFreshProcess
1720
from typed_python import (
1821
UInt64, UInt32,
1922
ListOf, TupleOf, Tuple, NamedTuple, Dict, OneOf, Forward, identityHash,
2023
Entrypoint, Class, Member, Final, TypeFunction, SerializationContext,
2124
Function, NotCompiled
2225
)
2326

24-
import typed_python
25-
2627
from typed_python.hash import Hash
2728

2829
from typed_python._types import (
@@ -112,6 +113,33 @@ def g(x: int):
112113
raise Exception("hash instability found")
113114

114115

116+
moduleLevelThreadLocal = threading.local()
117+
118+
119+
@NotCompiled
120+
def functionAccessingModuleLevelThreadLocal():
121+
return hasattr(moduleLevelThreadLocal, "anything")
122+
123+
124+
def test_identity_of_function_accessing_thread_local():
125+
print(typesAndObjectsVisibleToCompilerFrom(type(functionAccessingModuleLevelThreadLocal)))
126+
127+
identityHash(functionAccessingModuleLevelThreadLocal)
128+
moduleLevelThreadLocal.anything = True
129+
identityHash(functionAccessingModuleLevelThreadLocal)
130+
131+
hashInstability = checkForHashInstability()
132+
133+
if hashInstability is not None:
134+
print(hashInstability)
135+
raise Exception("hash instability found")
136+
137+
138+
def test_identity_of_method_descriptors():
139+
assert identityHash(ListOf(int).append) != identityHash(ListOf(float).append)
140+
assert identityHash(ListOf(int).append) != identityHash(ListOf(int).extend)
141+
142+
115143
def test_class_and_held_class_in_group():
116144
class C(Class):
117145
pass

0 commit comments

Comments
 (0)