pybind / pybind/pybind11

[BUG]: Unexpected behavior for binding to a data member, which is returned-by-reference by a member function

Open
#4,858 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

triage
Dominant language
C++
Stars
18k
Forks
2.3k
Avg merge
5d 17h
Merged PRs (30d)
10

Description

Required prerequisites
What version (or hash if on master) of pybind11 are you using?

pybind11-2.11.1-py3-none-any.whl.metadata (copied from pip install output when executing pip install -e .)

This was done in MacOS Ventura 13.5.2.

Python 3.10.13 (main, Aug 24 2023, 22:36:46) [Clang 14.0.3 (clang-1403.0.22.14.1)] on darwin
Problem description

The problem happens when I wrap a class to Python where that class enclosed another class.

class P {
public:
    EnclosedClass& get();
    EnclosedClass enclosed;
};

When calling get(), sometimes the copy constructor of EnclosedClass is called and sometimes it isn't. It all depends on if there is a Python object holding the data member in Python (e.g., when a = P() is defined in Python, setting another variable b = a.enclosed or not affects the calls of copy constructor of EnclosedClass).

The above dependence on a Python object is unexpected. I would hope that the copy constructor is never called at all cases because the codes was returning a reference. Could someone please point me out which thing I have missed?

Reproducible example code

I pushed to this repo.

cpp_src/example.cpp

#include <pybind11/pybind11.h>
#include <iostream>

namespace py = pybind11;


class Enclosed {
public:
    Enclosed() { c = 4; }
    Enclosed(const Enclosed& other): c(other.c) {
        std::cout << "Enclosed copy constructor invoked!" << std::endl;
    } 
    int c;
};


class TestBind {
public:
    TestBind() : _enclosed() {
        _enclosed.c = 5;
    }
    Enclosed& get(int level) {
        std::cout << "TestBind::get() returns a reference." << std::endl;
        _enclosed.c += level;
        return _enclosed;
    };
    Enclosed _enclosed;
};


PYBIND11_MODULE(_example, m) {
    m.doc() = R"pbdoc(Buggy!)pbdoc";

    py::class_<Enclosed>(m, "Enclosed")
        .def(py::init<>())
        .def_readwrite("c", &Enclosed::c)
        ;

    py::class_<TestBind>(m, "TestBind")
        .def(py::init<>())
        .def_readonly("enclosed", &TestBind::_enclosed)
        .def("get", &TestBind::get)
        ;

    m.attr("__version__") = "dev";
}

tests/test_enc.py

# Run by `pytest .`
import myproj._example


def test_enclosed_class():
    a = myproj._example.TestBind()
    # b = a.enclosed
    enc = a.get(6)
    enc2 = a.get(6)
    assert a.enclosed.c == 17
    assert enc2.c == 17
    assert enc.c == 17


def test_enclosed_class_2():
    a = myproj._example.TestBind()
    b = a.enclosed
    enc = a.get(6)
    enc2 = a.get(6)
    assert a.enclosed.c == 17
    assert enc2.c == 17
    assert enc.c == 17
    # assert False.  # uncomment this line to see the print out that copy constructor of Enclosed is not called.

pyproject.toml

# Run in your virtual environment with `pip install -e .`
[build-system]
requires = ["scikit-build-core>=0.5.0", "pybind11==2.11.1"]
build-backend = "scikit_build_core.build"

[tool.scikit-build]
wheel.expand-macos-universal-tags = true

[project]
name = "myproj"
version = "0.1.0"
description="MYPROJ"
readme = "README.md"
authors = [{ name = "My Name", email = "me@email.com" }]
requires-python = ">=3.10"
classifiers = [
  "Development Status :: 3 - Alpha",
  # "License :: OSI Approved :: MIT License",
  "Programming Language :: Python :: 3",
  "Programming Language :: Python :: 3.8",
  "Programming Language :: Python :: 3.9",
  "Programming Language :: Python :: 3.10",
  "Programming Language :: Python :: 3.11",
  # "Operating System :: OS Independent",
]
urls = { Homepage = "https://nodarsensor.com" }
dependencies = [  # related to "requires" in setup.py.  See https://peps.python.org/pep-0621/#other-names-for-dependencies-optional-dependencies

]

# Originally called "extra-requires" but the python community decided to rename it.
# https://peps.python.org/pep-0621/#other-names-for-dependencies-optional-dependencies
[project.optional-dependencies]
dev = [
  "black",
  "isort",
  "mypy",
  "pre-commit",
  "pytest",
  "pybind11",
  "ipython >= 8.15.0",
  "jupyter >= 1.0.0",
  "notebook >= 7.0.3",
]
all = [
  "myproj[dev]",
]

CMakeLists.txt

cmake_minimum_required(VERSION 3.15...3.26)

# Setup the cmake environment
# dependencies required for python binding.
find_package(Python REQUIRED COMPONENTS Interpreter Development.Module)
find_package(pybind11 CONFIG REQUIRED)


# Python project definition.
project(
  ${SKBUILD_PROJECT_NAME}
  VERSION ${SKBUILD_PROJECT_VERSION}
  LANGUAGES CXX
)


# Build a Python-readable cpp module.
## compile the python-readable cpp module.
set(PYTHON_MODULE_NAME _example)
python_add_library(${PYTHON_MODULE_NAME} MODULE cpp_src/example.cpp WITH_SOABI)
target_link_libraries(
    ${PYTHON_MODULE_NAME}
    PRIVATE
    pybind11::headers
)
target_compile_definitions(
    ${PYTHON_MODULE_NAME}
    PRIVATE
    VERSION_INFO=${PROJECT_VERSION}
)
## install the python-readable wrapper module
install(
    TARGETS ${PYTHON_MODULE_NAME} 
    DESTINATION ${SKBUILD_PROJECT_NAME}
)
Is this a regression? Put the last known working version here if it is.

Not a regression

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the reproducer in cpp_src/example.cpp and the two cases in tests/test_enc.py; build the extension using CMake and run pytest . Compare the behavior of get() with and without retaining a.enclosed. Done means establishing whether the copy-constructor behavior is a pybind11 bug or expected binding behavior, with the result captured in a regression test or clear explanation.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, python
Domain
api
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.