borglab / borglab/wrap

multiply inherited C++ classes

Open
#181 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
37
Forks
17
Avg merge
1d 4h
Merged PRs (30d)
9

Description

Title: pybind wrapper needs safe handling for inherited methods on multiply inherited C++ classes

Summary

We found a pybind wrapper bug affecting inherited non-virtual base methods when the wrapped C++ class has multiple inheritance but the generated binding only lists one base and does not use `py::multiple_inheritance()`.

Concrete case from GTSAM legged estimators:
- `gtsam::LeggedEstimator` declares non-virtual methods:
- `void turnHeightPriorOn(double terrainHeight);`
- `void turnHeightPriorOff();`
- `gtsam::LeggedInvariantEKF` inherits from:
- `LeftLinearEKF`
- `LeggedEstimator`

Observed behavior

When `turnHeightPriorOn/Off()` were exposed only on the abstract base `LeggedEstimator` in the `.i` file, Python calls on concrete `LeggedInvariantEKF` instances had no behavioral effect.

Workaround that fixed it immediately:
- redeclare the same methods on each concrete wrapped class in the `.i` file

That strongly suggests the issue is wrapper inheritance / base-pointer adjustment, not estimator logic.

pybind11 docs say this is undefined behavior unless:
- all C++ bases are listed, or
- `py::multiple_inheritance()` is supplied

Minimal reproduction pattern

- abstract base class with non-virtual method
- derived class with multiple C++ bases
- wrapper lists only one base
- base method is only bound on the base class
- calling that method from Python on a concrete object does not affect behavior correctly
- rebinding the same method directly on the concrete class works around the problem

The smallest fix that appears correct is:
- emit `py::multiple_inheritance()` for classes with a base class in generated pybind bindings

Possible better long-term fixes:
- model multiple bases explicitly in the interface language
- validate `.i` inheritance against the real C++ inheritance
- emit `py::multiple_inheritance()` only when truly needed, if that can be detected reliably

Regression test suggestion

Add a small pybind integration test with:
- base class method
- multiply inherited derived class
- binding lists one base
- verify that inherited method on derived object behaves correctly only when `py::multiple_inheritance()` is present

Why this matters

Without this fix, downstream projects have to duplicate inherited methods on concrete classes in `.i` files as a workaround, which is brittle and obscures the real wrapper problem.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.