Skip to content

[smart_holder] git merge master #4227

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 19 commits into from
Oct 10, 2022
Merged

Conversation

rwgk
Copy link
Collaborator

@rwgk rwgk commented Oct 7, 2022

Description

Helper/scratch PR for testing.

Suggested changelog entry:

Skylion007 and others added 14 commits September 19, 2022 12:56
* chore: Delete copy ctor/assign for GIL RAIIs

* Fix typo

* Delete copy ops for local gil scoped acquire
updates:
- [github.com/asottile/pyupgrade: v2.37.3 → v2.38.0](asottile/pyupgrade@v2.37.3...v2.38.0)
- [github.com/PyCQA/pylint: v2.15.2 → v2.15.3](pylint-dev/pylint@v2.15.2...v2.15.3)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Fix for windows compiler, missing object initializer

* Removal of if-else macro for MSVC
* Avoid local_internals destruction

It allows to avoid possible static deinitialization fiasco.

* Add link to relevant google style guide discussion
* Call reserve method in set and map casters too

* Refactor template logic into has_reserve_method

* Adjust comment for reviews

* Rearrange reserve_maybe to not be underneath macro
updates:
- [github.com/asottile/pyupgrade: v2.38.0 → v2.38.2](asottile/pyupgrade@v2.38.0...v2.38.2)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
updates:
- [github.com/pre-commit/mirrors-mypy: v0.971 → v0.981](pre-commit/mirrors-mypy@v0.971...v0.981)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Work-Around: NVCC 11.4.0 - 11.8.0

Adds a targeted NVCC work around for limited number of CUDA
releases. Fixed in NVCC development.

* style: pre-commit fixes

* CI: Bump CTK Version 11.2 -> 11.7

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…ybind#4217)

* Disable test triggering ASAN failure (to pin-point where the problem is).

* Fix unsafe "block" implementation in test_eigen.cpp

* Undo changes (i.e. revert back to master).

* Detect "type_caster for Eigen::Ref made a copy."

This is achieved without
* reaching into internals,
* making test_eigen.cpp depend on pybind11/numpy.h.

* Add comment pointing to PR, for easy reference.
pybind#4221)

* fix: PyCapsule_GetDestructor is allowed to return a nullptr destructor

Previously, this code would error out if the destructor happened to be
a nullptr. This is incorrect. nullptrs are allowed for capsule
destructors.

"It is legal for a capsule to have a NULL destructor. This makes a
NULL return code somewhat ambiguous; use PyCapsule_IsValid() or
PyErr_Occurred() to disambiguate."

See:

https://docs.python.org/3/c-api/capsule.html#c.PyCapsule_GetDestructor

I noticed this while working on a type caster related to pybind#3858 DLPack
happens to allow the destructor not to be defined on a capsule, and I
encountered such a case. See:

https://github.com/dmlc/dlpack/blob/e2bdd3bee8cb6501558042633fa59144cc8b7f5f/include/dlpack/dlpack.h#L219

* Add test for the fix.

* Update tests/test_pytypes.cpp

I tried this locally and it works!
I never knew that there are cases where `reinterpret_cast` does not work but `static_cast` does. Let's see if all compilers are happy with this.

Co-authored-by: Aaron Gokaslan <[email protected]>

* style: pre-commit fixes

Co-authored-by: Ralf W. Grosse-Kunstleve <[email protected]>
Co-authored-by: Ralf W. Grosse-Kunstleve <[email protected]>
Co-authored-by: Aaron Gokaslan <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
rwgk added 5 commits October 9, 2022 21:50
…ind#4228)

* Reproducer for issue encountered in smart_holder update.

* clang-tidy compatibility (untested).

* Add `enable_if_t` to workaround.

* Bug fix: Move `PYBIND11_USING_WORKAROUND_FOR_CUDA_11_4_THROUGH_8` determination to detail/common.h

So that it actually is defined in pybind11.h

* Try using the workaround (which is nicer than the original code) universally.

* Reduce reproducer for CUDA 11.7 issue encountered in smart_holder update.

This commit tested in isolation on top of current master + first version of reproducer (62311eb).

Succeeds with Debian Clang 14.0.6 C++17 (and probably all other compilers).

Fails for CUDA 11.7:

```
cd /build/tests && /usr/local/cuda/bin/nvcc -forward-unknown-to-host-compiler -Dpybind11_tests_EXPORTS -I/mounted_pybind11/include -isystem=/usr/include/python3.10 -g --generate-code=arch=compute_52,code=[compute_52,sm_52] -Xcompiler=-fPIC -Xcompiler=-fvisibility=hidden -Werror all-warnings -std=c++17 -MD -MT tests/CMakeFiles/pybind11_tests.dir/test_class.cpp.o -MF CMakeFiles/pybind11_tests.dir/test_class.cpp.o.d -x cu -c /mounted_pybind11/tests/test_class.cpp -o CMakeFiles/pybind11_tests.dir/test_class.cpp.o
/mounted_pybind11/tests/test_class.cpp(53): error: more than one instance of overloaded function "pybind11::class_<type_, options...>::def [with type_=test_class::pr4220_tripped_over_this::Empty0, options=<>]" matches the argument list:
            function template "pybind11::class_<test_class::pr4220_tripped_over_this::Empty0> &pybind11::class_<type_, options...>::def(const char *, Func &&, const Extra &...) [with type_=test_class::pr4220_tripped_over_this::Empty0, options=<>]"
/mounted_pybind11/include/pybind11/pybind11.h(1557): here
            function template "pybind11::class_<test_class::pr4220_tripped_over_this::Empty0> &pybind11::class_<type_, options...>::def(const T &, const Extra &...) [with type_=test_class::pr4220_tripped_over_this::Empty0, options=<>]"
/mounted_pybind11/include/pybind11/pybind11.h(1586): here
            argument types are: (const char [8], <unknown-type>)
            object type is: pybind11::class_<test_class::pr4220_tripped_over_this::Empty0>

1 error detected in the compilation of "/mounted_pybind11/tests/test_class.cpp".
```
```
SKIPPED [1] ../../mounted_pybind11/tests/test_type_caster_odr_guard_1.py:46: type_caster_odr_violation_detected_count() == 0: 11.2.0, C++17, CUDACC = 11.7.99
```
@rwgk
Copy link
Collaborator Author

rwgk commented Oct 10, 2022

The force push pushed ~exactly the same state of the code as before (except for a timestamp diff in .github/workflows/ci_sh_def.yml.patch), but with a cleaned-up commit history.

All code changes passed Google-global testing already (internal testing ID OCL:479646682:BASE:479875412:1665299298882:8037b7aa).

@rwgk rwgk merged commit 0aa8c94 into pybind:smart_holder Oct 10, 2022
@rwgk rwgk deleted the sh_merge_master branch October 10, 2022 05:11
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Oct 10, 2022
@rwgk rwgk removed the needs changelog Possibly needs a changelog entry label Oct 10, 2022
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.

7 participants