Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: ndarray
-
Labels:None
-
Story Points:1
-
Epic Link:
-
Sprint:DRP F18-4
-
Team:Data Release Production
Description
4/ndarray/1.5.1.lsst1+2/include/ndarray/pybind11.h:83:8: warning: 'ndarray::Pybind11Helper<double, 2, 2>' declared with greater visibility than the type of its field 'ndarray::Pybind11Helper<double, 2, 2>::wrapper' [-Wattributes] |
/home/jbosch/LSST/lsstsw/stack/Linux64/ndarray/1.5.1.lsst1+2/include/ndarray/pybind11.h: In instantiation of 'struct ndarray::Pybind11Helper<double, 1, 0>': |
/usr/include/c++/7/tuple:185:12: recursively required from 'struct std::_Tuple_impl<1, pybind11::detail::type_caster<ndarray::Array<double, 1, 0>, void>, pybind11::detail::type_caster<ndarray::Array<double, 2, 0>, void>, pybind11::detail::type_caster<lsst::geom::Point<int, 2>, void> >' |
I had thought these would go away with DM-15151, but I guess not, and it's probably better for us to fix them in the code anyway.
Attachments
Issue Links
- relates to
-
- Done
Activity
This looks fine. Thanks for fixing it.
One small question: would it make any sense to always hide symbol visibility for those classes? It feels a bit fragile to depend on undocumented behavior of pybind11 (that it uses dunder-GNUC to decide if it should hide symbols).
If we wanted to hide them for any other compilers (note that both gcc and clang with define _GNUG_), we'd have to know the compiler-specific way to do that. The only other one I know the incantation for is Microsoft, but pybind11 doesn't even use that one, so it doesn't seem right for ndarray to do it in its pybind11 bindings either.
Russell Owen, mind a quick review? (I know you're supposed to be doing T&S things now, but you have way more context here than anyone else, so this should be a super fast for review for you but a slow one for anyone else)
lsst/ndarray changes are just an updated tarball, + an unrelated tiny Doxygen build fix I've been meaning to get around to.
Upstream changes are already merged and released, but you can take a look at https://github.com/ndarray/ndarray/commit/c4c6e5906cfa9bd1a8e33b2b9dffe9355c16ede2