Skip to content

Added a test to detect invalid RTTI caching #409

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.

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
merged 1 commit into from
Sep 11, 2016

Conversation

jagerman
Copy link
Member

The current inheritance testing isn't sufficient to detect a cache failure; the test added here breaks PR #390, which caches the run-time-determined return type the first time a function is called, then reuses that cached type even though the run-time type could be different for a future call.

The current inheritance testing isn't sufficient to detect a cache
failure; the test added here breaks PR pybind#390, which caches the
run-time-determined return type the first time a function is called,
then reuses that cached type even though the run-time type could be
different for a future call.
@wjakob
Copy link
Member

wjakob commented Sep 11, 2016

LGTM

@wjakob wjakob merged commit 591a999 into pybind:master Sep 11, 2016
@jagerman jagerman deleted the dynamic-cast-test branch September 11, 2016 23:39
jagerman added a commit to jagerman/pybind11 that referenced this pull request Sep 12, 2016
Each time a function is called, we currently do a type lookup in
registered_types_cpp to figure out the python type info associated with
the return value.  This lookup isn't free, particularly for often-called
functions, but will usually yield an identical result each time the
function is called.

This commit adds a per-return-type cache into the return type lookup for
generic types (i.e. those inheriting from type_caster_generic) to avoid
the hash lookup.  The cache is invalidated whenever the runtime type
changes (to avoid the problem PR pybind#409 tests for).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants