Repository navigation
[cpyrt] Deduce a template instantiation per annotated Python callable - #154
guitargeek wants to merge 1 commit into
Conversation
| return name; | ||
| } | ||
|
|
||
| static bool AnnotationsMatch(PyObject* pyobject, const std::string& rettype, |
There was a problem hiding this comment.
Can we avoid string comparisons and hook to the underlying clang/CppInterOp types. That will give us some level of memoization stability where we can cache the actual function prototype from the AST.
There was a problem hiding this comment.
Done. The converter now holds the parameter's AST type and the annotated signature is resolved to a type (cached per spelling), compared with IsSimilarFnTypes. Added a test with a typedef annotation that the string comparison got wrong.
Vipul-Cariappa
left a comment
Vipul-Cariappa
left a comment
There was a problem hiding this comment.
Please look at commit 425cada in #150 for my approach to fix this. It is a more concise fix than this.
The underlying issue is that the hash for the cache is the same for all different Python functions. This is fixed by taking the annotation into account while computing the hash; if the annotation is not available, then we use the Python function object itself as the hash.
| # an unannotated callable is wrapped by the known instantiation | ||
| assert ns.call(D) == 2 |
There was a problem hiding this comment.
This may not be supported with the new overload resolution. We do not have type information on either the Python or C++ side.
There was a problem hiding this comment.
Thanks for the heads up! That line records the behavior on main today. It can be dropped together with the new overload resolution if that no longer supports it.
An implicit template call with an annotated Python function deduced the template argument from the annotations only the first time. A later call with a differently annotated callable reused that first instantiation, silently wrapping the new callable with the old signature (e.g. truncating a float argument to int). The signature hash behind the memoized dispatch only mixes in the Python type, which all functions share, and the function pointer converter wrapped any callable regardless of its annotations. Mix the annotation values of Python functions and bound methods into HashSignature. Give the function pointer converter the type it converts to and have it, in the strict round, accept an annotated callable only if the annotations declare that function type: the declared signature is resolved to a type once per spelling and compared through CppInterOp, so the spelling need not match the parameter's. A mismatch defers the callable to the implicit round, as for other implicit conversions. The std::function converter skips the check, as a std::function takes any compatible callable. A None annotation now reads as void. Fixes compiler-research#151
09d1368 to
34f16c5
Compare
This branch already hashes the annotations into |
|
What happens on a hash collision? Do we call the wrong thing? |
This branch has not been deployed
3 participants
An implicit template call with an annotated Python function deduced the template argument from the annotations only the first time. A later call with a differently annotated callable reused that first instantiation, silently wrapping the new callable with the old signature (e.g. truncating a float argument to int). The signature hash behind the memoized dispatch only mixes in the Python type, which all functions share, and the function pointer converter wrapped any callable regardless of its annotations.
Mix the annotation values of Python functions and bound methods into HashSignature. Give the function pointer converter the type it converts to and have it, in the strict round, accept an annotated callable only if the annotations declare that function type: the declared signature is resolved to a type once per spelling and compared through CppInterOp, so the spelling need not match the parameter's. A mismatch defers the callable to the implicit round, as for other implicit conversions. The std::function converter skips the check, as a std::function takes any compatible callable. A None annotation now reads as void.
Fixes #151