mirror of
https://github.com/pybind/pybind11.git
synced 2024-11-15 09:54:48 +00:00
79fd12b8cc
14 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Ralf W. Grosse-Kunstleve
|
79fd12b8cc |
Add NOLINTNEXTLINE(bugprone-unused-return-value) in smart_holder_poc_test.cpp
Follow-on to https://github.com/pybind/pybind11/pull/5272 — clang-tidy upgrade (to version 18) Fixes this error: ``` /__w/pybind11/pybind11/tests/pure_cpp/smart_holder_poc_test.cpp:167:12: error: the value returned by this function should not be disregarded; neglecting it may lead to errors [bugprone-unused-return-value,-warnings-as-errors] 167 | (void) new_owner.release(); // Manually verified: without this, clang++ -fsanitize=address | ^~~~~~~~~~~~~~~~~~~ ``` See also: https://clang.llvm.org/extra/clang-tidy/checks/bugprone/unused-return-value.html Specifically there: > std::unique_ptr::release(). Not using the return value can lead to resource leaks if the same pointer isn’t stored anywhere else. Often, ignoring the release() return value indicates that the programmer confused the function with reset(). I.e. the only way to tell clang-tidy that the smart_holder_poc_test.cpp is correct is to add the `NOLINT`. |
||
|
Ralf W. Grosse-Kunstleve
|
55105fbeaf
|
[smart_holder] Bug fix: std::unique_ptr deleter needs to be copied. (#4850)
* Store `std::function<void (void *)>` del_fun; in `guarded_delete`
* Specialize the simple common case.
Using a `union` is complicated: https://en.cppreference.com/w/cpp/language/union
> If members of a union are classes with user-defined constructors and destructors, to switch the active member, explicit destructor and placement new are generally needed:
Using `std::variant` increases compile-time overhead.
It is currently unclear how much these effects matter in practice: optimization left for later.
* Add one test case (more later).
* Add `const` to resolve clang-tidy error.
```
-- The CXX compiler identification is Clang 15.0.7
/usr/bin/cmake -E __run_co_compile --tidy="/usr/bin/clang-tidy;--use-color;--warnings-as-errors=*;--extra-arg-before=--driver-mode=g++" --source=/__w/pybind11/pybind11/tests/test_class_sh_inheritance.cpp -- /usr/bin/c++ -DPYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD_IF_AVAILABLE -DPYBIND11_TEST_EIGEN -Dpybind11_tests_EXPORTS -I/__w/pybind11/pybind11/include -isystem /usr/include/python3.9 -isystem /__w/pybind11/pybind11/build/_deps/eigen-src -Os -DNDEBUG -fPIC -fvisibility=hidden -Wall -Wextra -Wconversion -Wcast-qual -Wdeprecated -Wundef -Wnon-virtual-dtor -flto=thin -std=c++17 -o CMakeFiles/pybind11_tests.dir/test_class_sh_inheritance.cpp.o -c /__w/pybind11/pybind11/tests/test_class_sh_inheritance.cpp
/__w/pybind11/pybind11/tests/pure_cpp/smart_holder_poc_test.cpp:264:30: error: pointer parameter 'raw_ptr' can be pointer to const [readability-non-const-parameter,-warnings-as-errors]
new int(19), [](int *raw_ptr) { delete raw_ptr; });
^
const
```
* Introduce `struct custom_deleter` to ensure the deleter is moved as desired (the lambda function only captures a reference, which can become dangling).
* Resolve helpful clang-tidy errors.
```
/usr/bin/cmake -E __run_co_compile --tidy="/usr/bin/clang-tidy;--use-color;--warnings-as-errors=*;--extra-arg-before=--driver-mode=g++" --source=/__w/pybind11/pybind11/tests/test_class.cpp -- /usr/bin/c++ -DPYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD_IF_AVAILABLE -DPYBIND11_TEST_EIGEN -Dpybind11_tests_EXPORTS -I/__w/pybind11/pybind11/include -isystem /usr/include/python3.9 -isystem /__w/pybind11/pybind11/build/_deps/eigen-src -Os -DNDEBUG -fPIC -fvisibility=hidden -Wall -Wextra -Wconversion -Wcast-qual -Wdeprecated -Wundef -Wnon-virtual-dtor -flto=thin -std=c++17 -o CMakeFiles/pybind11_tests.dir/test_class.cpp.o -c /__w/pybind11/pybind11/tests/test_class.cpp
/__w/pybind11/pybind11/include/pybind11/detail/smart_holder_poc.h:114:5: error: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor,-warnings-as-errors]
custom_deleter(D &&deleter) : deleter{std::move(deleter)} {}
^
explicit
/__w/pybind11/pybind11/include/pybind11/detail/smart_holder_poc.h:120:76: error: forwarding reference passed to std::move(), which may unexpectedly cause lvalues to be moved; use std::forward() instead [bugprone-move-forwarding-reference,-warnings-as-errors]
return guarded_delete(std::function<void(void *)>(custom_deleter<T, D>(std::move(uqp_del))),
^~~~~~~~~
std::forward<D>
```
* Workaround for gcc 4.8.5, clang 3.6
* Transfer reduced test here.
Reduced from a PyCLIF use case in the wild by @wangxf123456 (internal change cl/565476030).
* Add missing include (clangd Include Cleaner)
* Change `std::move` to `std::forward` as suggested by @iwanders.
* Add missing includes (clangd Include Cleaner)
* Use new `PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP` to exclude `smart_holder::as_unique_ptr` method from production code.
* Systematically add `PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP` to mark code that is not used from production code. Add comment to explain.
* Very simple experiment related to https://github.com/pybind/pybind11/pull/4850#issuecomment-1789780676
Does the `PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP` define have anything to do with it?
* Revert "Very simple experiment related to https://github.com/pybind/pybind11/pull/4850#issuecomment-1789780676"
This reverts commit
|
||
|
Ralf W. Grosse-Kunstleve
|
8f31c19e99 | Fix new (after upgrade) clang-tidy error, adjust .codespell-ignorelines accordingly. | ||
Xiaofei Wang
|
c0cfe95b1b
|
Support loading unique_ptr<Derived> as unique_ptr<Base>. (#4031)
* Support loading unique_ptr<derived> as unique_ptr<base>. * Fix incorrect test * pre commit fix * Fix clang tidy * Resolve comments * Resolve comments |
||
|
Ralf W. Grosse-Kunstleve
|
97862b126a
|
Using #undef _ before "#include <catch.hpp>", as suggested by @henryiii (#3747)
|
||
|
Ralf W. Grosse-Kunstleve
|
b4e1ac9a94 | clang-tidy fixes related to PR #3250 | ||
|
Ralf W. Grosse-Kunstleve
|
e41fb99e7e | clang-tidy fixes (mostly manual) related to PR #3166 | ||
|
Ralf W. Grosse-Kunstleve
|
52c3f4cc30
|
This was meant to be PR #3065: pure clang-format changes. NO manual changes. (#3073) | ||
|
Ralf W. Grosse-Kunstleve
|
2c828ff552 |
More clang-tidy fixes. These escaped before because -DDOWNLOAD_EIGEN=ON -DDOWNLOAD_CATCH=ON -DCMAKE_CXX_STANDARD=17, as used in the GitHub Actions, were missing in the interactive run.
|
||
|
Ralf W. Grosse-Kunstleve
|
6c922614ed
|
Adding reclaim_disowned logic & miscellaneous naming and documentation improvements. (#2943)
* Using new smart_holder::reclaim_disowned in smart_holder_type_caster for unique_ptr. * Systematically renaming was_disowned to is_disowned (because disowning is now reversible: reclaim_disowned). * Systematically renaming virtual_overrider_self_life_support to trampoline_self_life_support (to reuse existing terminology instead of introducing new one). * Systematically renaming test_class_sh_with_alias to test_class_sh_trampoline_basic. * Adding a Trampolines and std::unique_ptr section to README_smart_holder.rst. * MSVC compatibility. |
||
|
Ralf W. Grosse-Kunstleve
|
469792032a
|
Adding virtual_overrider_self_life_support. (#2902)
* Initial version of virtual_overrider_self_life_support (enables safely passing unique_ptr to C++). * Clang 3.6, 3.7 compatibility. * Adding missing default constructor. * Restoring test for exception for the case that virtual_overrider_self_life_support is not used. * Fixing oversight: Adding missing holder().ensure_was_not_disowned(). * Adding unit tests for new `struct smart_holder` member functions. * Moving virtual_overrider_self_life_support to separate include file, with iwyu cleanup. |
||
|
Ralf W. Grosse-Kunstleve
|
3a336a2047
|
shared_ptr<bool> vptr_deleter_armed_flag_ptr (instead of unique_ptr) (#2882)
* shared_ptr<bool> vptr_deleter_armed_flag_ptr (instead of unique_ptr), to fix heap-use-after-free bug. * Fixing generated by some compilers in the pybind11 CI suite. |
||
|
Ralf W. Grosse-Kunstleve
|
01e0045547
|
Enabling use of smart_holder for types with non-public destructors. (#2878)
* Enabling use of smart_holder for types with non-public destructors. * Resolving clang-tidy error (GitHub CI). |
||
|
Ralf W. Grosse-Kunstleve
|
1bafd5db5f
|
Adding py::smart_holder (for smart-pointer interoperability). (#2672)
* Adding test_unique_ptr_member (for desired PyCLIF behavior). See also: https://github.com/pybind/pybind11/issues/2583 Does not build with upstream master or https://github.com/pybind/pybind11/pull/2047, but builds with https://github.com/RobotLocomotion/pybind11 and almost runs: ``` Running tests in directory "/usr/local/google/home/rwgk/forked/EricCousineau-TRI/pybind11/tests": ================================================================================= test session starts ================================================================================= platform linux -- Python 3.8.5, pytest-5.4.3, py-1.9.0, pluggy-0.13.1 rootdir: /usr/local/google/home/rwgk/forked/EricCousineau-TRI/pybind11/tests, inifile: pytest.ini collected 2 items test_unique_ptr_member.py .F [100%] ====================================================================================== FAILURES ======================================================================================= _____________________________________________________________________________ test_pointee_and_ptr_owner ______________________________________________________________________________ def test_pointee_and_ptr_owner(): obj = m.pointee() assert obj.get_int() == 213 m.ptr_owner(obj) with pytest.raises(ValueError) as exc_info: > obj.get_int() E Failed: DID NOT RAISE <class 'ValueError'> test_unique_ptr_member.py:17: Failed ============================================================================= 1 failed, 1 passed in 0.06s ============================================================================= ``` * unique_ptr or shared_ptr return * new test_variant_unique_shared with vptr_holder prototype * moving prototype code to pybind11/vptr_holder.h, adding type_caster specialization to make the bindings involving unique_ptr passing compile, but load and cast implementations are missing * disabling GitHub Actions on pull_request (for this PR) * disabling AppVeyor (for this PR) * TRIGGER_SEGSEV macro, annotations for GET_STACK (vptr::get), GET_INT_STACK (pointee) * adding test_promotion_of_disowned_to_shared * Copying tests as-is from xxx_value_ptr_xxx_holder branch. https://github.com/rwgk/pybind11/tree/xxx_value_ptr_xxx_holder Systematically exercising returning and passing unique_ptr<T>, shared_ptr<T> with unique_ptr, shared_ptr holder. Observations: test_holder_unique_ptr: make_unique_pointee OK pass_unique_pointee BUILD_FAIL (as documented) make_shared_pointee Abort free(): double free detected pass_shared_pointee RuntimeError: Unable to load a custom holder type from a default-holder instance test_holder_shared_ptr: make_unique_pointee Segmentation fault (#1138) pass_unique_pointee BUILD_FAIL (as documented) make_shared_pointee OK pass_shared_pointee OK * Copying tests as-is from xxx_value_ptr_xxx_holder branch. https://github.com/rwgk/pybind11/tree/xxx_value_ptr_xxx_holder Systematically exercising casting between shared_ptr<base>, shared_ptr<derived>. * Demonstration of Undefined Behavior in handling of shared_ptr holder. Based on https://godbolt.org/z/4fdjaW by jorgbrown@ (thanks Jorg!). * Additional demonstration of Undefined Behavior in handling of shared_ptr holder. * fixing up-down mixup in comment * Demonstration of Undefined Behavior in handling of polymorphic pointers. (This demo does NOT involve smart pointers at all, unlike the otherwise similar test_smart_ptr_private_first_base.) * minor test_private_first_base.cpp simplification (after discovering that this can be wrapped with Boost.Python, using boost::noncopyable) * pybind11 equivalent of Boost.Python test similar to reproducer under #1333 * Snapshot of WIP, TODO: shared_ptr deleter with on/off switch * Adding vptr_deleter. * Adding from/as unique_ptr<T> and unique_ptr<T, D>. * Adding from_shared_ptr. Some polishing. * New tests/core/smart_holder_poc_test.cpp, using Catch2. * Adding in vptr_deleter_guard_flag. * Improved labeling of TEST_CASEs. * Shuffling existing TEST_CASEs into systematic matrix. * Implementing all [S]uccess tests. * Implementing all [E]xception tests. * Testing of exceptions not covered by the from-as matrix. * Adding top-level comment. * Converting from methods to factory functions (no functional change). * Removing obsolete and very incomplete test (replaced by Catch2-based test). * Removing stray file. * Adding type_caster_bare_interface_demo. * Adding shared_ptr<mpty>, shared_ptr<mpty const> casters. * Adding unique_ptr<mpty>, unique_ptr<mpty const> casters. * Pure copy of `class class_` implementation in pybind11.h (master commit |