Use stricter brace initialization

This updates the `py::init` constructors to only use brace
initialization for aggregate initiailization if there is no constructor
with the given arguments.

This, in particular, fixes the regression in #1247 where the presence of
a `std::initializer_list<T>` constructor started being invoked for
constructor invocations in 2.2 even when there was a specific
constructor of the desired type.

The added test case demonstrates: without this change, it fails to
compile because the `.def(py::init<std::vector<int>>())` constructor
tries to invoke the `T(std::initializer_list<std::vector<int>>)`
constructor rather than the `T(std::vector<int>)` constructor.

By only using `new T{...}`-style construction when a `T(...)`
constructor doesn't exist, we should bypass this by while still allowing
`py::init<...>` to be used for aggregate type initialization (since such
types, by definition, don't have a user-declared constructor).

Author: jagerman

include/pybind11/detail/init.h

@@ -52,6 +52,16 @@ bool is_alias(Cpp<Class> *ptr) {
 template <typename /*Class*/>
 constexpr bool is_alias(void *) { return false; }
 
+// Constructs and returns a new object; if the given arguments don't map to a constructor, we fall
+// back to brace aggregate initiailization so that for aggregate initialization can be used with
+// py::init, e.g.  `py::init<int, int>` to initialize a `struct T { int a; int b; }`.  For
+// non-aggregate types, we need to use an ordinary T(...) constructor (invoking as `T{...}` usually
+// works, but will not do the expected thing when `T` has an `initializer_list<T>` constructor).
+template <typename Class, typename... Args, detail::enable_if_t<std::is_constructible<Class, Args...>::value, int> = 0>
+inline Class *construct_or_initialize(Args &&...args) { return new Class(std::forward<Args>(args)...); }
+template <typename Class, typename... Args, detail::enable_if_t<!std::is_constructible<Class, Args...>::value, int> = 0>
+inline Class *construct_or_initialize(Args &&...args) { return new Class{std::forward<Args>(args)...}; }
+
 // Attempts to constructs an alias using a `Alias(Cpp &&)` constructor.  This allows types with
 // an alias to provide only a single Cpp factory function as long as the Alias can be
 // constructed from an rvalue reference of the base Cpp type.  This means that Alias classes
@@ -161,7 +171,7 @@ struct constructor {
     template <typename Class, typename... Extra, enable_if_t<!Class::has_alias, int> = 0>
     static void execute(Class &cl, const Extra&... extra) {
         cl.def("__init__", [](value_and_holder &v_h, Args... args) {
-            v_h.value_ptr() = new Cpp<Class>{std::forward<Args>(args)...};
+            v_h.value_ptr() = construct_or_initialize<Cpp<Class>>(std::forward<Args>(args)...);
         }, is_new_style_constructor(), extra...);
     }
 
@@ -171,9 +181,9 @@ struct constructor {
     static void execute(Class &cl, const Extra&... extra) {
         cl.def("__init__", [](value_and_holder &v_h, Args... args) {
             if (Py_TYPE(v_h.inst) == v_h.type->type)
-                v_h.value_ptr() = new Cpp<Class>{std::forward<Args>(args)...};
+                v_h.value_ptr() = construct_or_initialize<Cpp<Class>>(std::forward<Args>(args)...);
             else
-                v_h.value_ptr() = new Alias<Class>{std::forward<Args>(args)...};
+                v_h.value_ptr() = construct_or_initialize<Alias<Class>>(std::forward<Args>(args)...);
         }, is_new_style_constructor(), extra...);
     }
 
@@ -182,7 +192,7 @@ struct constructor {
                           !std::is_constructible<Cpp<Class>, Args...>::value, int> = 0>
     static void execute(Class &cl, const Extra&... extra) {
         cl.def("__init__", [](value_and_holder &v_h, Args... args) {
-            v_h.value_ptr() = new Alias<Class>{std::forward<Args>(args)...};
+            v_h.value_ptr() = construct_or_initialize<Alias<Class>>(std::forward<Args>(args)...);
         }, is_new_style_constructor(), extra...);
     }
 };
@@ -193,7 +203,7 @@ template <typename... Args> struct alias_constructor {
               enable_if_t<Class::has_alias && std::is_constructible<Alias<Class>, Args...>::value, int> = 0>
     static void execute(Class &cl, const Extra&... extra) {
         cl.def("__init__", [](value_and_holder &v_h, Args... args) {
-            v_h.value_ptr() = new Alias<Class>{std::forward<Args>(args)...};
+            v_h.value_ptr() = construct_or_initialize<Alias<Class>>(std::forward<Args>(args)...);
         }, is_new_style_constructor(), extra...);
     }
 };

tests/test_class.cpp

@@ -10,6 +10,16 @@
 #include "pybind11_tests.h"
 #include "constructor_stats.h"
 #include "local_bindings.h"
+#include <pybind11/stl.h>
+
+// test_brace_initialization
+struct NoBraceInitialization {
+    NoBraceInitialization(std::vector<int> v) : vec{std::move(v)} {}
+    template <typename T>
+    NoBraceInitialization(std::initializer_list<T> l) : vec(l) {}
+
+    std::vector<int> vec;
+};
 
 TEST_SUBMODULE(class_, m) {
     // test_instance
@@ -299,6 +309,12 @@ TEST_SUBMODULE(class_, m) {
         .def(py::init<int, const std::string &>())
         .def_readwrite("field1", &BraceInitialization::field1)
         .def_readwrite("field2", &BraceInitialization::field2);
+    // We *don't* want to construct using braces when the given constructor argument maps to a
+    // constructor, because brace initialization could go to the wrong place (in particular when
+    // there is also an `initializer_list<T>`-accept constructor):
+    py::class_<NoBraceInitialization>(m, "NoBraceInitialization")
+        .def(py::init<std::vector<int>>())
+        .def_readonly("vec", &NoBraceInitialization::vec);
 
     // test_reentrant_implicit_conversion_failure
     // #1035: issue with runaway reentrant implicit conversion

tests/test_class.py

@@ -228,6 +228,12 @@ def test_brace_initialization():
     assert a.field1 == 123
     assert a.field2 == "test"
 
+    # Tests that a non-simple class doesn't get brace initialization (if the
+    # class defines an initializer_list constructor, in particular, it would
+    # win over the expected constructor).
+    b = m.NoBraceInitialization([123, 456])
+    assert b.vec == [123, 456]
+
 
 @pytest.unsupported_on_pypy
 def test_class_refcount():