Extra any_container constructor

[mglisse]

Description

This lets us construct an array with

py::array_t<double>({ 2L, 3 });

without needing to first cast everything to the same type. Note that it will not allow mixed types if narrowing conversions are involved, but that's not what I am aiming for right now.

It is similar to #885, but that PR was replacing the template constructor with this one, while I am adding it as a complement. This avoids introducing narrowing errors, since those cases can keep using the old constructor.

(Obviously this isn't a priority, just a nice-to-have if it does not cause trouble elsewhere)

[rwgk]

• Looks good to me.
• The github CI is completely happy.
• I'll run this through Google-testing when I get a chance (hopefully soon).

[rwgk]

I finally got around to running this PR through our global testing. Unfortunately I'm seeing hundreds of build breakages like this:

xxx/yyy/zzz/file.cc:125:35: error: braces around scalar initializer [-Werror,-Wbraced-scalar-init]
                py::array_t<int>({{env.batch_size()}}, env.step_type().data(),
                                  ^~~~~~~~~~~~~~~~~~

Taking that as a measure for how much trouble this will cause in the world at large, I think this change is too disruptive.

I didn't think this through at all, sorry if this is naive: is there a way to not break the {{...}} initializers while still enabling the mixed type initializer?

[mglisse]

Taking that as a measure for how much trouble this will cause in the world at large, I think this change is too disruptive.

Ok, thanks for testing. AFAIU those warnings are harmless and only produced by some very old version of clang, but I understand they can be an issue for some users.

I didn't think this through at all, sorry if this is naive: is there a way to not break the {{...}} initializers while still enabling the mixed type initializer?

I'll think about it. In the mean time, let me switch this PR to draft (or maybe close it).

[rwgk]

I took a slightly closer look at the ~640 build failures. According to some tool we have, there are only 3 root causes, i.e. the build breakages may actually be easy to fix, at least internally (inside Google). External breakages are a different problem: the same bindings code would need to work with the pybind11 version we have internally (smart_holder branch, with is closely tracking master) and externally with a couple pybind11 releases back. Do you think that's doable? — I'm still having doubts about cost-vs-benefit in this particular case, but please let us know your own assessment.

We are using a super-fresh version of clang internally (we follow LLVM upstream OSS very closely), with -Wbraced-scalar-init centrally configured as build-breaking error. Overriding the global options is only a last resort (for good reasons IMO).

[mglisse]

I don't think the current patch is worth the trouble, unless we can tweak it so it avoids this clang warning without modifying callers (I haven't had time to experiment with that).

[mglisse]

It seems that it is really by accident that clang does not warn with the current pybind11 code.

#include <initializer_list>

struct A{
#ifdef WARN
  A(std::initializer_list<int>);
#else
  template<class T>A(std::initializer_list<T>);
#endif
};
void f(A){}

void g(int i){f({{i}});}

Clang warns if I define WARN, but not otherwise. But T is deduced as int, so we are really doing the same thing in both cases. However, clang has

// Don't warn during template instantiation. If the initialization was
// non-dependent, we warned during the initial parse; otherwise, the
// type might not be scalar in some uses of the template.

which disables the warning too broadly I think. (I didn't step through it, maybe the warning is disabled by something else)

The double braces make perfect sense if we expect to use the constructor any_container(std::vector<T> &&v). And I actually now realize that I should have used double braces in the initial code: py::array_t<double>({{ 2L, 3 }}); already works fine by calling the constructor that takes a vector, no need to patch anything. I don't know if there is a nice place to document that...

[rwgk]

no need to patch anything.

Cool :-)

I don't know if there is a nice place to document that...

My feeling is that explaining such a nuance in the documentation is easily more confusing than helpful, but a well-crafted concise comment in a unit test is pretty likely to work as intended. (That's where I look for nuances.) Could that make sense here? Do you want to reuse this PR for a small unit test addition?