Skip to content
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

Detect and fail if using mismatched holders #2644

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Commits on Nov 6, 2020

  1. Configuration menu
    Copy the full SHA
    b498e52 View commit details
    Browse the repository at this point in the history
  2. Detect and fail if using mismatched holders

    This adds a check when registering a class or a function with a holder
    return that the same wrapped type hasn't been previously seen using a
    different holder type.
    
    This fixes pybind#1138 by detecting the failure; currently attempting to use
    two different holder types (e.g. a unique_ptr<T> and shared_ptr<T>) in
    difference places can segfault because we don't have any type safety on
    the holder instances.
    jagerman authored and rhaschke committed Nov 6, 2020
    Configuration menu
    Copy the full SHA
    44f23a2 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    0fe5697 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    6360006 View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    e2aa4ec View commit details
    Browse the repository at this point in the history
  6. fixup! Replace global map holders_seen with local holder_type in regi…

    …stered type_info
    
    Having replaced the global map holders_seen, we can only check return values against already
    registered types. Hence, we need to replace the check at initialization time with a check at call
    time (when casting the return value).
    rhaschke committed Nov 6, 2020
    Configuration menu
    Copy the full SHA
    502bf24 View commit details
    Browse the repository at this point in the history
  7. Generalize holder compatibility check for derived classes

    A derived class needs to use the same holder type as its base class(es).
    So far, the check was constrained to the default holder vs. custom holder types.
    Thus, we replace the simple check (based on the default_holder flag) with
    a more elaborate one, comparing holder type names.
    rhaschke committed Nov 6, 2020
    Configuration menu
    Copy the full SHA
    7852e7d View commit details
    Browse the repository at this point in the history
  8. Relax holder type check

    rhaschke committed Nov 6, 2020
    Configuration menu
    Copy the full SHA
    cde5c11 View commit details
    Browse the repository at this point in the history

Commits on Nov 8, 2020

  1. fixup! Replace global map holders_seen with local holder_type in regi…

    …stered type_info
    
    Improve wording and variable name:
    - seen_holder_name -> holder_name
    - seen holder -> declared holder
    rhaschke committed Nov 8, 2020
    Configuration menu
    Copy the full SHA
    d5c4386 View commit details
    Browse the repository at this point in the history
  2. fixup! fixup! Replace global map holders_seen with local holder_type …

    …in registered type_info
    
    Runtime costs of checking holder compatibility for function return types during can be limited to
    definition time (instead of calling time), if we require types to be registered before defining a function.
    This allows to lookup the type_info record and validate the holder type as expected.
    As we cannot call functions with an unregistered return type anyway, I think this is a good compromise.
    rhaschke committed Nov 8, 2020
    Configuration menu
    Copy the full SHA
    e57a3db View commit details
    Browse the repository at this point in the history

Commits on Nov 9, 2020

  1. fixup! fixup! fixup! Replace global map holders_seen with local holde…

    …r_type in registered type_info
    
    Simplify code using type_id<Return>()
    rhaschke committed Nov 9, 2020
    Configuration menu
    Copy the full SHA
    8115384 View commit details
    Browse the repository at this point in the history

Commits on Nov 23, 2020

  1. Move runtime check for holder compatibility into function registration

    To avoid costly runtime checks at function call time, compatibility of
    holder types is now checked once at functionn registration time.
    This assumes that all custom argument types are declared in advance!
    rhaschke committed Nov 23, 2020
    Configuration menu
    Copy the full SHA
    02d0b53 View commit details
    Browse the repository at this point in the history