Skip to content

Commit

Permalink
Fix Attribute copy/move constructors (#1545)
Browse files Browse the repository at this point in the history
* Add failing test that Attribute constructors work properly

* Ensure that constructor doesnt override default constructors

* Use macro instead
  • Loading branch information
franzpoeschel authored Nov 7, 2023
1 parent ac31667 commit 9ad33b0
Show file tree
Hide file tree
Showing 4 changed files with 158 additions and 5 deletions.
113 changes: 113 additions & 0 deletions include/openPMD/DatatypeMacros.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/* Copyright 2023 Franz Poeschel
*
* This file is part of openPMD-api.
*
* openPMD-api is free software: you can redistribute it and/or modify
* it under the terms of of either the GNU General Public License or
* the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* openPMD-api is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License and the GNU Lesser General Public License
* for more details.
*
* You should have received a copy of the GNU General Public License
* and the GNU Lesser General Public License along with openPMD-api.
* If not, see <http://www.gnu.org/licenses/>.
*/
#pragma once

#include <array>
#include <complex>
#include <vector>

// Need to alias this to avoid the comma
// Cannot use a namespace, otherwise the macro will work either only within
// or outside the namespace
// Defining macros means polluting the global namespace anyway
using openpmd_array_double_7 = std::array<double, 7>;

#define OPENPMD_FOREACH_DATATYPE(MACRO) \
MACRO(char) \
MACRO(unsigned char) \
MACRO(signed char) \
MACRO(short) \
MACRO(int) \
MACRO(long) \
MACRO(long long) \
MACRO(unsigned short) \
MACRO(unsigned int) \
MACRO(unsigned long) \
MACRO(unsigned long long) \
MACRO(float) \
MACRO(double) \
MACRO(long double) \
MACRO(std::complex<float>) \
MACRO(std::complex<double>) \
MACRO(std::complex<long double>) \
MACRO(std::string) \
MACRO(std::vector<char>) \
MACRO(std::vector<short>) \
MACRO(std::vector<int>) \
MACRO(std::vector<long>) \
MACRO(std::vector<long long>) \
MACRO(std::vector<unsigned char>) \
MACRO(std::vector<unsigned short>) \
MACRO(std::vector<unsigned int>) \
MACRO(std::vector<unsigned long>) \
MACRO(std::vector<unsigned long long>) \
MACRO(std::vector<float>) \
MACRO(std::vector<double>) \
MACRO(std::vector<long double>) \
MACRO(std::vector<std::complex<float>>) \
MACRO(std::vector<std::complex<double>>) \
MACRO(std::vector<std::complex<long double>>) \
MACRO(std::vector<signed char>) \
MACRO(std::vector<std::string>) \
MACRO(openpmd_array_double_7) \
MACRO(bool)

#define OPENPMD_FOREACH_NONVECTOR_DATATYPE(MACRO) \
MACRO(char) \
MACRO(unsigned char) \
MACRO(signed char) \
MACRO(short) \
MACRO(int) \
MACRO(long) \
MACRO(long long) \
MACRO(unsigned short) \
MACRO(unsigned int) \
MACRO(unsigned long) \
MACRO(unsigned long long) \
MACRO(float) \
MACRO(double) \
MACRO(long double) \
MACRO(std::complex<float>) \
MACRO(std::complex<double>) \
MACRO(std::complex<long double>) \
MACRO(std::string) \
MACRO(array_double_7) \
MACRO(bool)

#define OPENPMD_FOREACH_DATASET_DATATYPE(MACRO) \
MACRO(char) \
MACRO(unsigned char) \
MACRO(signed char) \
MACRO(short) \
MACRO(int) \
MACRO(long) \
MACRO(long long) \
MACRO(unsigned short) \
MACRO(unsigned int) \
MACRO(unsigned long) \
MACRO(unsigned long long) \
MACRO(float) \
MACRO(double) \
MACRO(long double) \
MACRO(std::complex<float>) \
MACRO(std::complex<double>) \
MACRO(std::complex<long double>) \
MACRO(std::array<double, 7>)
24 changes: 24 additions & 0 deletions include/openPMD/UndefDatatypeMacros.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/* Copyright 2023 Franz Poeschel
*
* This file is part of openPMD-api.
*
* openPMD-api is free software: you can redistribute it and/or modify
* it under the terms of of either the GNU General Public License or
* the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* openPMD-api is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License and the GNU Lesser General Public License
* for more details.
*
* You should have received a copy of the GNU General Public License
* and the GNU Lesser General Public License along with openPMD-api.
* If not, see <http://www.gnu.org/licenses/>.
*/

#undef OPENPMD_FOREACH_DATATYPE
#undef OPENPMD_FOREACH_NONVECTOR_DATATYPE
#undef OPENPMD_FOREACH_DATASET_DATATYPE
18 changes: 13 additions & 5 deletions include/openPMD/backend/Attribute.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#pragma once

#include "openPMD/Datatype.hpp"
#include "openPMD/DatatypeMacros.hpp"
#include "openPMD/auxiliary/TypeTraits.hpp"
#include "openPMD/auxiliary/Variant.hpp"

Expand Down Expand Up @@ -85,9 +86,9 @@ class Attribute
std::vector<float>,
std::vector<double>,
std::vector<long double>,
std::vector<std::complex<float> >,
std::vector<std::complex<double> >,
std::vector<std::complex<long double> >,
std::vector<std::complex<float>>,
std::vector<std::complex<double>>,
std::vector<std::complex<long double>>,
std::vector<signed char>,
std::vector<std::string>,
std::array<double, 7>,
Expand All @@ -106,10 +107,15 @@ class Attribute
*
* Fix by explicitly instantiating resource
*/
template <typename T>
Attribute(T &&val) : Variant(resource(std::forward<T>(val)))

#define OPENPMD_ATTRIBUTE_CONSTRUCTOR_FROM_VARIANT(TYPE) \
Attribute(TYPE val) : Variant(resource(std::move(val))) \
{}

OPENPMD_FOREACH_DATATYPE(OPENPMD_ATTRIBUTE_CONSTRUCTOR_FROM_VARIANT)

#undef OPENPMD_ATTRIBUTE_CONSTRUCTOR_FROM_VARIANT

/** Retrieve a stored specific Attribute and cast if convertible.
*
* @note This performs a static_cast and might introduce precision loss if
Expand Down Expand Up @@ -297,3 +303,5 @@ std::optional<U> Attribute::getOptional() const
std::move(eitherValueOrError));
}
} // namespace openPMD

#include "openPMD/UndefDatatypeMacros.hpp"
8 changes: 8 additions & 0 deletions test/CoreTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ TEST_CASE("attribute_dtype_test", "[core]")
{
Attribute a = Attribute(static_cast<char>(' '));
REQUIRE(Datatype::CHAR == a.dtype);
{
// check that copy constructor works
[[maybe_unused]] Attribute b = a;
}
{
// check that move constructor works
[[maybe_unused]] Attribute b = std::move(a);
}
a = Attribute(static_cast<unsigned char>(' '));
REQUIRE(Datatype::UCHAR == a.dtype);
a = Attribute(static_cast<short>(0));
Expand Down

0 comments on commit 9ad33b0

Please sign in to comment.