Merge 'upstream/master' (07e2259) from previous merge-base (12e8774) #34
Conversation
* fix: Avoid conversion to `int_` rhs argument of enum eq/ne * test: compare unscoped enum with strings * suppress comparison to None warning * test unscoped enum arithmetic and comparision with unsupported type
[ci skip]
* Adapt to python3.8 C API change Do `Py_DECREF(type)` on all python objects on deallocation fix pybind#1946 * Add bare python3.8 build to CI matrix While numpy/scipy wheels are available, run python3.8 test without them
When building with `-Werror,-Wmissing-prototypes`, `clang` complains about missing prototypes for functions defined through macro expansions. This PR adds the missing prototypes.
```
error: no previous prototype for function 'pybind11_init_impl_embedded' [
-Werror,-Wmissing-prototypes]
PYBIND11_EMBEDDED_MODULE(embedded, mod) {
^
external/pybind11/include/pybind11/embed.h:61:5: note: expanded from macro 'PYBIND11_EMBEDDED_MODULE'
PYBIND11_EMBEDDED_MODULE_IMPL(name) \
^
external/pybind11/include/pybind11/embed.h:26:23: note: expanded from macro 'PYBIND11_EMBEDDED_MODULE_IMPL'
extern "C" void pybind11_init_impl_##name() { \
^
<scratch space>:380:1: note: expanded from here
pybind11_init_impl_embedded
^
1 error generated.
```
… number of entries) (pybind#1958)
Don't assume that just because the language version is C++17 that the standard library offers all C++17 features, too. When using clang-6.0 and --std=c++17 on Ubuntu 18.04 with libstdc++, __cpp_sized_deallocation is false.
* test pair-copyability on C++17 upwards The stdlib falsely detects containers like M=std::map<T, U> as copyable, even when one of T and U is not copyable. Therefore we cannot rely on the stdlib dismissing std::pair<T, M> by itself, even on C++17. * fix is_copy_assignable bind_map used std::is_copy_assignable which suffers from the same problems as std::is_copy_constructible, therefore the same fix has been applied. * created tests for copyability
Add Python 3.8 to considered versions in CMake for additional hints. https://cmake.org/cmake/help/v3.2/module/FindPythonLibs.html
This reverts commit 819802d.
If a debug interpreter is detected, we allow linking with pythonXX_d.lib under windows.
* Fix test build in C++20 * Add C++20 char8_t/u8string support
…bind#2043) This commit introduces the use of C++17-style fold expressions when casting tuples & the argument lists of functions. This change can improve performance of the resulting bindings: because fold expressions have short-circuiting semantics, pybind11 e.g. won't try to cast the second argument of a function if the first one failed. This is particularly effective when working with functions that have many overloads with long argument lists.
When binding code immediately throws an exception of type py::error_already_set (e.g. via py::module::import that fails), the catch block sets an import error as expected. Unfortunately, following this, the deconstructor of py::error_already_set decides to call py::detail::get_internals() and set up various internal data structures of pybind11, which fails given that the error flag is active. The call stack of this looks as follows: Py_init_mymodule() -> __cxa_decrement_exception_refcount -> error_already_set::~error_already_set() -> gil_scoped_acquire::gil_scoped_acquire() -> detail::get_internals() -> ... -> pybind11::detail::simple_collector() -> uh oh.. The solution is simple: we call detail::get_internals() once before running any binding code to make sure that the internal data structures are ready.
There was a problem hiding this comment.
+@jwnimmer-tri for review, please?
At a high-level is, this all seems like bug and aesthetic fixes (esp. the import error_already_set part), with one feature on readonly buffers.
Reviewable status: 0 of 35 files reviewed, all discussions resolved (waiting on @jwnimmer-tri)
|
Neither https://github.com/RobotLocomotion/drake/tree/master/tools/workspace/pybind11 nor https://github.com/RobotLocomotion/pybind11/tree/master nor https://github.com/RobotLocomotion/pybind11/tree/drake nor any other branch appears to explain the git strategy we use to maintain the fork. I don't need spoon-fed command lines, but I do need a few sentences about our approach and which branch names and/or tags in our repo are relevant vs strays. I can settle for a discussion thread in the PR here with that detail, to be merged into a docs tree somewhere later. Or does it exist already and I overlooked it? |
|
(And as far as code review, I don't care about reviewing the diffs at all -- once we have the branch policy and CI checks enumerated, review should be as simple as comparing the PR vs those items. We don't need to re-review upstream.) |
|
@BetsyMcPhail Thanks! Merged it in. @jwnimmer-tri It does not exist yet - sorry about that! I will follow-up after this PR and merge in some instructions that are hopefully easy to discover. We started out with a rebasing Git merge strategy, but I found that to be a bit burdensome. I then switched to a merge strategy, and that seems fine. The checklist outlined above is a modified form of what I've done on prior PRs. Some notes on branches / tags:
Are you OK with the current merge strategy, with the promise of me cleaning this up afterwards? |
# Conflicts: # include/pybind11/cast.h # include/pybind11/pybind11.h
EricCousineau-TRI
force-pushed
the
feature-merge_2020-01-13
branch
from
9f7bed6 to
6ca4163
Compare
So #36 is great, thanks. It should give me enough context to review this PR. However, the PR description checklist still has these items in it:
Are they still part of the plan? They are not listed as steps in #36. |
|
However, I do not believe that we should do any of the other git items in the top post. |
|
Sounds good to me. Will strike those out. |
There was a problem hiding this comment.
Reviewed 35 of 35 files at r1.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @jwnimmer-tri)
|
Failure on CI is simply a deadsnakes PPA flake. Going to merge. |
Process:
Merge olddrakeintono_prunewithmerge --no-ff --strategy=oursResetlast_sha_of_previous_forkto olddrake, force pushResetupstream, push without--forceThis change is