Skip to content

Commit

Permalink
Python: Fix ODR Violation (#1521)
Browse files Browse the repository at this point in the history
In pybind11 (and nanobind), auxiliary headers and
`PYBIND11_MAKE_OPAQUE` definitions for distributed modules need to be
included in every translation unit. Otherwise, a one-definition-rule
violation occurs, which is undefined behavior.
  • Loading branch information
ax3l authored Aug 20, 2023
1 parent 9d5bf96 commit acf1c11
Show file tree
Hide file tree
Showing 26 changed files with 113 additions and 182 deletions.
55 changes: 55 additions & 0 deletions include/openPMD/binding/python/Common.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/* Copyright 2023 The openPMD Community
*
* This header is used to centrally define classes that shall not violate the
* C++ one-definition-rule (ODR) for various Python translation units.
*
* Authors: Axel Huebl
* License: LGPL-3.0-or-later
*/
#pragma once

#include "openPMD/Iteration.hpp"
#include "openPMD/Mesh.hpp"
#include "openPMD/ParticlePatches.hpp"
#include "openPMD/ParticleSpecies.hpp"
#include "openPMD/Record.hpp"
#include "openPMD/Series.hpp"
#include "openPMD/backend/BaseRecord.hpp"
#include "openPMD/backend/BaseRecordComponent.hpp"
#include "openPMD/backend/MeshRecordComponent.hpp"
#include "openPMD/backend/PatchRecord.hpp"
#include "openPMD/backend/PatchRecordComponent.hpp"

#include <pybind11/gil.h>
#include <pybind11/numpy.h>
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>
#include <pybind11/stl_bind.h>
// not yet used:
// pybind11/functional.h // for std::function

// used exclusively in all our Python .cpp files
namespace py = pybind11;
using namespace openPMD;

// opaque types
using PyIterationContainer = Series::IterationsContainer_t;
using PyMeshContainer = Container<Mesh>;
using PyPartContainer = Container<ParticleSpecies>;
using PyPatchContainer = Container<ParticlePatches>;
using PyRecordContainer = Container<Record>;
using PyPatchRecordContainer = Container<PatchRecord>;
using PyRecordComponentContainer = Container<RecordComponent>;
using PyMeshRecordComponentContainer = Container<MeshRecordComponent>;
using PyPatchRecordComponentContainer = Container<PatchRecordComponent>;
using PyBaseRecordComponentContainer = Container<BaseRecordComponent>;
PYBIND11_MAKE_OPAQUE(PyIterationContainer)
PYBIND11_MAKE_OPAQUE(PyMeshContainer)
PYBIND11_MAKE_OPAQUE(PyPartContainer)
PYBIND11_MAKE_OPAQUE(PyPatchContainer)
PYBIND11_MAKE_OPAQUE(PyRecordContainer)
PYBIND11_MAKE_OPAQUE(PyPatchRecordContainer)
PYBIND11_MAKE_OPAQUE(PyRecordComponentContainer)
PYBIND11_MAKE_OPAQUE(PyMeshRecordComponentContainer)
PYBIND11_MAKE_OPAQUE(PyPatchRecordComponentContainer)
PYBIND11_MAKE_OPAQUE(PyBaseRecordComponentContainer)
4 changes: 1 addition & 3 deletions include/openPMD/binding/python/Numpy.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@

#include "openPMD/Datatype.hpp"

#include <pybind11/numpy.h>
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>
#include "Common.hpp"

#include <exception>
#include <string>
Expand Down
5 changes: 1 addition & 4 deletions include/openPMD/binding/python/Pickle.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@
#include "openPMD/Series.hpp"
#include "openPMD/backend/Attributable.hpp"

#include <pybind11/pybind11.h>
#include <pybind11/stl.h>
#include "Common.hpp"

#include <exception>
#include <string>
Expand All @@ -46,8 +45,6 @@ template <typename... T_Args, typename T_SeriesAccessor>
inline void
add_pickle(pybind11::class_<T_Args...> &cl, T_SeriesAccessor &&seriesAccessor)
{
namespace py = pybind11;

// helper: get first class in py::class_ - that's the type we pickle
using PickledClass =
typename std::tuple_element<0, std::tuple<T_Args...> >::type;
Expand Down
6 changes: 1 addition & 5 deletions src/binding/python/Access.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,9 @@
* and the GNU Lesser General Public License along with openPMD-api.
* If not, see <http://www.gnu.org/licenses/>.
*/
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

#include "openPMD/IO/Access.hpp"

namespace py = pybind11;
using namespace openPMD;
#include "openPMD/binding/python/Common.hpp"

void init_Access(py::module &m)
{
Expand Down
10 changes: 2 additions & 8 deletions src/binding/python/Attributable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,9 @@
#include "openPMD/DatatypeHelpers.hpp"
#include "openPMD/auxiliary/Variant.hpp"
#include "openPMD/backend/Attribute.hpp"
#include "openPMD/binding/python/Numpy.hpp"

#include <pybind11/pybind11.h>
#include <pybind11/stl.h>
#include <pybind11/stl_bind.h>
#include "openPMD/binding/python/Common.hpp"
#include "openPMD/binding/python/Numpy.hpp"

#include <algorithm>
#include <array>
Expand All @@ -35,11 +33,7 @@
#include <string>
#include <vector>

namespace py = pybind11;
using namespace openPMD;

using PyAttributeKeys = std::vector<std::string>;
// PYBIND11_MAKE_OPAQUE(PyAttributeKeys)

bool setAttributeFromBufferInfo(
Attributable &attr, std::string const &key, py::buffer &a)
Expand Down
8 changes: 2 additions & 6 deletions src/binding/python/BaseRecord.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,14 @@
* and the GNU Lesser General Public License along with openPMD-api.
* If not, see <http://www.gnu.org/licenses/>.
*/
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

#include "openPMD/backend/BaseRecord.hpp"
#include "openPMD/backend/BaseRecordComponent.hpp"
#include "openPMD/backend/Container.hpp"
#include "openPMD/backend/MeshRecordComponent.hpp"
#include "openPMD/backend/PatchRecordComponent.hpp"
#include "openPMD/binding/python/UnitDimension.hpp"

namespace py = pybind11;
using namespace openPMD;
#include "openPMD/binding/python/Common.hpp"
#include "openPMD/binding/python/UnitDimension.hpp"

void init_BaseRecord(py::module &m)
{
Expand Down
11 changes: 3 additions & 8 deletions src/binding/python/BaseRecordComponent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,14 @@
* and the GNU Lesser General Public License along with openPMD-api.
* If not, see <http://www.gnu.org/licenses/>.
*/
#include <pybind11/numpy.h>
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

#include "openPMD/Datatype.hpp"
#include "openPMD/backend/BaseRecordComponent.hpp"
#include "openPMD/Datatype.hpp"

#include "openPMD/binding/python/Common.hpp"
#include "openPMD/binding/python/Numpy.hpp"

#include <sstream>

namespace py = pybind11;
using namespace openPMD;

void init_BaseRecordComponent(py::module &m)
{
py::class_<BaseRecordComponent, Attributable>(m, "Base_Record_Component")
Expand Down
8 changes: 2 additions & 6 deletions src/binding/python/ChunkInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,13 @@
* and the GNU Lesser General Public License along with openPMD-api.
* If not, see <http://www.gnu.org/licenses/>.
*/
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

#include "openPMD/ChunkInfo.hpp"

#include "openPMD/binding/python/Common.hpp"

#include <exception>
#include <string>

namespace py = pybind11;
using namespace openPMD;

void init_Chunk(py::module &m)
{
py::class_<ChunkInfo>(m, "ChunkInfo")
Expand Down
42 changes: 2 additions & 40 deletions src/binding/python/Container.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,33 +23,16 @@
*
* BSD-style license, see pybind11 LICENSE file.
*/

#include <pybind11/pybind11.h>
#include <pybind11/stl.h>
#include <pybind11/stl_bind.h>

#include "openPMD/Iteration.hpp"
#include "openPMD/Mesh.hpp"
#include "openPMD/ParticlePatches.hpp"
#include "openPMD/ParticleSpecies.hpp"
#include "openPMD/Record.hpp"
#include "openPMD/Series.hpp"
#include "openPMD/backend/BaseRecord.hpp"
#include "openPMD/backend/BaseRecordComponent.hpp"
#include "openPMD/backend/Container.hpp"
#include "openPMD/backend/MeshRecordComponent.hpp"
#include "openPMD/backend/PatchRecord.hpp"
#include "openPMD/backend/PatchRecordComponent.hpp"

#include "openPMD/binding/python/Common.hpp"

#include <cstddef>
#include <memory>
#include <sstream>
#include <string>
#include <utility>

namespace py = pybind11;
using namespace openPMD;

namespace detail
{
/* based on std_bind.h in pybind11
Expand Down Expand Up @@ -156,27 +139,6 @@ bind_container(py::handle scope, std::string const &name, Args &&...args)
}
} // namespace detail

using PyIterationContainer = Series::IterationsContainer_t;
using PyMeshContainer = Container<Mesh>;
using PyPartContainer = Container<ParticleSpecies>;
using PyPatchContainer = Container<ParticlePatches>;
using PyRecordContainer = Container<Record>;
using PyPatchRecordContainer = Container<PatchRecord>;
using PyRecordComponentContainer = Container<RecordComponent>;
using PyMeshRecordComponentContainer = Container<MeshRecordComponent>;
using PyPatchRecordComponentContainer = Container<PatchRecordComponent>;
using PyBaseRecordComponentContainer = Container<BaseRecordComponent>;
PYBIND11_MAKE_OPAQUE(PyIterationContainer)
PYBIND11_MAKE_OPAQUE(PyMeshContainer)
PYBIND11_MAKE_OPAQUE(PyPartContainer)
PYBIND11_MAKE_OPAQUE(PyPatchContainer)
PYBIND11_MAKE_OPAQUE(PyRecordContainer)
PYBIND11_MAKE_OPAQUE(PyPatchRecordContainer)
PYBIND11_MAKE_OPAQUE(PyRecordComponentContainer)
PYBIND11_MAKE_OPAQUE(PyMeshRecordComponentContainer)
PYBIND11_MAKE_OPAQUE(PyPatchRecordComponentContainer)
PYBIND11_MAKE_OPAQUE(PyBaseRecordComponentContainer)

void init_Container(py::module &m)
{
::detail::bind_container<PyIterationContainer>(m, "Iteration_Container");
Expand Down
8 changes: 2 additions & 6 deletions src/binding/python/Dataset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,13 @@
* and the GNU Lesser General Public License along with openPMD-api.
* If not, see <http://www.gnu.org/licenses/>.
*/
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

#include "openPMD/Dataset.hpp"

#include "openPMD/binding/python/Common.hpp"
#include "openPMD/binding/python/Numpy.hpp"

#include <string>

namespace py = pybind11;
using namespace openPMD;

void init_Dataset(py::module &m)
{
py::class_<Dataset>(m, "Dataset")
Expand Down
8 changes: 2 additions & 6 deletions src/binding/python/Datatype.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,10 @@
* and the GNU Lesser General Public License along with openPMD-api.
* If not, see <http://www.gnu.org/licenses/>.
*/
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

#include "openPMD/Datatype.hpp"
#include "openPMD/binding/python/Numpy.hpp"

namespace py = pybind11;
using namespace openPMD;
#include "openPMD/binding/python/Common.hpp"
#include "openPMD/binding/python/Numpy.hpp"

void init_Datatype(py::module &m)
{
Expand Down
13 changes: 9 additions & 4 deletions src/binding/python/Error.cpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
/* Copyright 2019-2023 The openPMD Community
*
* This header is used to centrally define classes that shall not violate the
* C++ one-definition-rule (ODR) for various Python translation units.
*
* Authors: Franz Poeschel, Axel Huebl
* License: LGPL-3.0-or-later
*/
#include "openPMD/Error.hpp"

#include <pybind11/pybind11.h>

namespace py = pybind11;
using namespace openPMD;
#include "openPMD/binding/python/Common.hpp"

void init_Error(py::module &m)
{
Expand Down
8 changes: 2 additions & 6 deletions src/binding/python/Helper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,16 @@
* and the GNU Lesser General Public License along with openPMD-api.
* If not, see <http://www.gnu.org/licenses/>.
*/
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

#include "openPMD/Series.hpp"
#include "openPMD/cli/ls.hpp"
#include "openPMD/helper/list_series.hpp"

#include "openPMD/binding/python/Common.hpp"

#include <sstream>
#include <string>
#include <vector>

namespace py = pybind11;
using namespace openPMD;

void init_Helper(py::module &m)
{
m.def(
Expand Down
8 changes: 2 additions & 6 deletions src/binding/python/Iteration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,14 @@
* and the GNU Lesser General Public License along with openPMD-api.
* If not, see <http://www.gnu.org/licenses/>.
*/
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

#include "openPMD/Iteration.hpp"

#include "openPMD/binding/python/Common.hpp"

#include <ios>
#include <sstream>
#include <string>

namespace py = pybind11;
using namespace openPMD;

void init_Iteration(py::module &m)
{
py::class_<Iteration, Attributable>(m, "Iteration")
Expand Down
6 changes: 1 addition & 5 deletions src/binding/python/IterationEncoding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,9 @@
* and the GNU Lesser General Public License along with openPMD-api.
* If not, see <http://www.gnu.org/licenses/>.
*/
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

#include "openPMD/IterationEncoding.hpp"

namespace py = pybind11;
using namespace openPMD;
#include "openPMD/binding/python/Common.hpp"

void init_IterationEncoding(py::module &m)
{
Expand Down
8 changes: 2 additions & 6 deletions src/binding/python/Mesh.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,17 @@
* and the GNU Lesser General Public License along with openPMD-api.
* If not, see <http://www.gnu.org/licenses/>.
*/
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

#include "openPMD/Mesh.hpp"
#include "openPMD/backend/BaseRecord.hpp"
#include "openPMD/backend/MeshRecordComponent.hpp"

#include "openPMD/binding/python/Common.hpp"
#include "openPMD/binding/python/Pickle.hpp"
#include "openPMD/binding/python/UnitDimension.hpp"

#include <string>
#include <vector>

namespace py = pybind11;
using namespace openPMD;

void init_Mesh(py::module &m)
{
py::class_<Mesh, BaseRecord<MeshRecordComponent> > cl(m, "Mesh");
Expand Down
Loading

0 comments on commit acf1c11

Please sign in to comment.