DM-15151: Set symbol visibility to hidden in pybind11 wrappers #39
Conversation
r-owen
force-pushed
the
tickets/DM-15151
branch
2 times, most recently
from
c1295ce
to
d2c1850
Compare
r-owen
changed the title
r-owen
force-pushed
the
tickets/DM-15151
branch
3 times, most recently
from
ed939e2
to
6aeb0f2
Compare
include/lsst/daf/base/PropertySet.h
Outdated
| * | ||
| * Implemented in the .cc file to work around symbol visiblity issues on macOS | ||
| * e.g. https://github.com/pybind/pybind11/issues/1503 | ||
| */ |
There was a problem hiding this comment.
The location of the implementation doesn't seem like something we want in the API documentation. Consider moving to a normal comment.
There was a problem hiding this comment.
Should this be a method on PropertySet in the first place? If it's only needed for pybind11, maybe it should be a separate function (in the same files as now, I take it?).
There was a problem hiding this comment.
The implementation has to be in the shared object library that contains PropertySet. I don't think it's possible to do that if I only define the function in the pybind11 wrapper.
There is also a bit of C++ code that uses typeOfT (in pex_policy), though there may be another way to do it.
I will change the comment as per your suggestion.
| [](std::type_info const& self, std::type_info const& other) { return self != other; }); | ||
| [](std::type_info const& self, std::type_info const& other) { return self != other; }) | ||
| .def("name", &std::type_info::name) | ||
| .def("hash_code", &std::type_info::hash_code); |
There was a problem hiding this comment.
I fear that will make it harder to find (I only added these fields for debugging). But what the heck, I'll change it. If folks really want it they can read the wrapper.
r-owen
force-pushed
the
tickets/DM-15151
branch
from
fcc2a33
to
890fd78
Compare
it help debug issues in Python
The way _propertyContainerGet got a PropertySet or PropertyList was by first trying to return as a PropertyList, then checking the type and returning as a PropertySet if it matched. This was inefficient because the type of a PropertyList is the same as a PropertySet, so do that check first. Also, use isPropertySetPtr for the test to make the code look a bit cleaner.
r-owen
force-pushed
the
tickets/DM-15151
branch
from
890fd78
to
9a3d969
Compare
|
Having ripped out boost persistence in DM-15599 the symbol export is no longer necessary. I will remove it assuming a full Jenkins run (including ci_hsc) passes. |
No description provided.