From 92dceaa28cde1d811d98b1f60cc0d260e4cf41ac Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Sun, 9 Sep 2018 13:07:26 +0200 Subject: [PATCH] Datatype operator== Besides returning true for the same types, identical implementations on some platforms, e.g. if long and long long are the same or double and long double will also return true. Affected by https://stackoverflow.com/questions/44515148/why-is-operator-overload-of-enum-ambiguous-in-msvc on MSVC. --- include/openPMD/Datatype.hpp | 63 ++++++++++++++++++++++++++ src/Iteration.cpp | 1 + src/RecordComponent.cpp | 13 +++--- test/CoreTest.cpp | 28 ++++++++++++ test/SerialIOTest.cpp | 86 +++++++++++++++++++++++++++++++----- 5 files changed, 174 insertions(+), 17 deletions(-) diff --git a/include/openPMD/Datatype.hpp b/include/openPMD/Datatype.hpp index f04548ac8d..c069a2d7dd 100644 --- a/include/openPMD/Datatype.hpp +++ b/include/openPMD/Datatype.hpp @@ -472,8 +472,71 @@ isSameInteger( Datatype d ) else return false; } + +/** Comparison for two Datatypes + * + * Besides returning true for the same types, identical implementations on + * some platforms, e.g. if long and long long are the same or double and + * long double will also return true. + */ +inline bool +isSame( openPMD::Datatype const d, openPMD::Datatype const e ) +{ + // exact same type + if( static_cast(d) == static_cast(e) ) + return true; + + bool d_is_vec = isVector( d ); + bool e_is_vec = isVector( e ); + + // same int + bool d_is_int, d_is_sig; + std::tie(d_is_int, d_is_sig) = isInteger( d ); + bool e_is_int, e_is_sig; + std::tie(e_is_int, e_is_sig) = isInteger( e ); + if( + d_is_int && + e_is_int && + d_is_vec == e_is_vec && + d_is_sig == e_is_sig && + toBits( d ) == toBits( e ) + ) + return true; + + // same float + bool d_is_fp = isFloatingPoint( d ); + bool e_is_fp = isFloatingPoint( e ); + + if( + d_is_fp && + e_is_fp && + d_is_vec == e_is_vec && + toBits( d ) == toBits( e ) + ) + return true; + + return false; +} } // namespace openPMD +#if !defined(_MSC_VER) +/** Comparison Operator for Datatype + * + * Overwrite the builtin default comparison which would only match exact same + * Datatype. + * + * Broken in MSVC < 19.11 (before Visual Studio 2017.3) + * https://stackoverflow.com/questions/44515148/why-is-operator-overload-of-enum-ambiguous-in-msvc + * + * @see openPMD::isSame + */ +inline bool +operator==( openPMD::Datatype d, openPMD::Datatype e ) +{ + return openPMD::isSame(d, e); +} +#endif + namespace std { ostream& diff --git a/src/Iteration.cpp b/src/Iteration.cpp index 2953838d13..93cb08bd69 100644 --- a/src/Iteration.cpp +++ b/src/Iteration.cpp @@ -21,6 +21,7 @@ #include #include "openPMD/auxiliary/StringManip.hpp" #include "openPMD/Dataset.hpp" +#include "openPMD/Datatype.hpp" #include "openPMD/Iteration.hpp" #include "openPMD/Series.hpp" diff --git a/src/RecordComponent.cpp b/src/RecordComponent.cpp index d37abbd5ba..4c4d24ffe3 100644 --- a/src/RecordComponent.cpp +++ b/src/RecordComponent.cpp @@ -224,13 +224,12 @@ RecordComponent::readBase() Extent e; // uint64_t check - Datatype attrDtype = *aRead.dtype; - if( isSameInteger< uint64_t >( attrDtype ) ) - if( isVector( attrDtype ) ) - for( auto const& val : getCast< std::vector< uint64_t > >( a ) ) - e.push_back( val ); - else - e.push_back( getCast< uint64_t >( a ) ); + Datatype const attrDtype = *aRead.dtype; + if( isSame( attrDtype, determineDatatype< uint64_t >() ) ) + e.push_back( getCast< uint64_t >( a ) ); + else if( isSame( attrDtype, determineDatatype< std::vector< uint64_t > >() ) ) + for( auto const& val : getCast< std::vector< uint64_t > >( a ) ) + e.push_back( val ); else { std::ostringstream oss; diff --git a/test/CoreTest.cpp b/test/CoreTest.cpp index 1a23d09b87..7e4c61eee5 100644 --- a/test/CoreTest.cpp +++ b/test/CoreTest.cpp @@ -96,6 +96,34 @@ TEST_CASE( "attribute_dtype_test", "[core]" ) a = Attribute(static_cast< uint64_t >(0)); REQUIRE(determineDatatype< uint64_t >() == a.dtype); // TODO fixed size floats + + // same implementation types (not necessary aliases) detection + if( sizeof(long) == sizeof(long long) ) + { + a = Attribute(static_cast< long >(0)); + REQUIRE(isSame(Datatype::LONGLONG, a.dtype)); +#if !defined(_MSC_VER) + REQUIRE(Datatype::LONGLONG == a.dtype); +#endif + a = Attribute(static_cast< long long >(0)); + REQUIRE(isSame(Datatype::LONG, a.dtype)); +#if !defined(_MSC_VER) + REQUIRE(Datatype::LONG == a.dtype); +#endif + } + if( sizeof(int) == sizeof(long) ) + { + a = Attribute(static_cast< long >(0)); + REQUIRE(isSame(Datatype::INT, a.dtype)); +#if !defined(_MSC_VER) + REQUIRE(Datatype::INT == a.dtype); +#endif + a = Attribute(static_cast< int >(0)); + REQUIRE(isSame(Datatype::LONG, a.dtype)); +#if !defined(_MSC_VER) + REQUIRE(Datatype::LONG == a.dtype); +#endif + } } TEST_CASE( "output_default_test", "[core]" ) diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index 636ba9c435..8dce728179 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -798,15 +798,24 @@ TEST_CASE( "hzdr_hdf5_sample_content_test", "[serial][hdf5]" ) PatchRecordComponent& e_extent_x = e_extent["x"]; REQUIRE(e_extent_x.unitSI() == 2.599999993753294e-07); - REQUIRE(isSameInteger< uint64_t >( e_extent_x.getDatatype() )); +#if !defined(_MSC_VER) + REQUIRE(e_extent_x.getDatatype() == determineDatatype< uint64_t >()); +#endif + REQUIRE(isSame(e_extent_x.getDatatype(), determineDatatype< uint64_t >())); PatchRecordComponent& e_extent_y = e_extent["y"]; REQUIRE(e_extent_y.unitSI() == 4.429999943501912e-08); - REQUIRE(isSameInteger< uint64_t >( e_extent_y.getDatatype() )); +#if !defined(_MSC_VER) + REQUIRE(e_extent_y.getDatatype() == determineDatatype< uint64_t >()); +#endif + REQUIRE(isSame(e_extent_y.getDatatype(), determineDatatype< uint64_t >())); PatchRecordComponent& e_extent_z = e_extent["z"]; REQUIRE(e_extent_z.unitSI() == 2.599999993753294e-07); - REQUIRE(isSameInteger< uint64_t >( e_extent_z.getDatatype() )); +#if !defined(_MSC_VER) + REQUIRE(e_extent_z.getDatatype() == determineDatatype< uint64_t >()); +#endif + REQUIRE(isSame(e_extent_z.getDatatype(), determineDatatype< uint64_t >())); std::shared_ptr< uint64_t > data; e_extent_z.load(0, data); @@ -818,7 +827,10 @@ TEST_CASE( "hzdr_hdf5_sample_content_test", "[serial][hdf5]" ) REQUIRE(e_numParticles.count(RecordComponent::SCALAR) == 1); PatchRecordComponent& e_numParticles_scalar = e_numParticles[RecordComponent::SCALAR]; - REQUIRE(isSameInteger< uint64_t >( e_numParticles_scalar.getDatatype() )); +#if !defined(_MSC_VER) + REQUIRE(e_numParticles_scalar.getDatatype() == determineDatatype< uint64_t >()); +#endif + REQUIRE(isSame(e_numParticles_scalar.getDatatype(), determineDatatype< uint64_t >())); e_numParticles_scalar.load(0, data); o.flush(); @@ -829,7 +841,10 @@ TEST_CASE( "hzdr_hdf5_sample_content_test", "[serial][hdf5]" ) REQUIRE(e_numParticlesOffset.count(RecordComponent::SCALAR) == 1); PatchRecordComponent& e_numParticlesOffset_scalar = e_numParticlesOffset[RecordComponent::SCALAR]; - REQUIRE(isSameInteger< uint64_t >( e_numParticlesOffset_scalar.getDatatype() )); +#if !defined(_MSC_VER) + REQUIRE(e_numParticlesOffset_scalar.getDatatype() == determineDatatype< uint64_t >()); +#endif + REQUIRE(isSame(e_numParticlesOffset_scalar.getDatatype(), determineDatatype< uint64_t >())); PatchRecord& e_offset = e_patches["offset"]; REQUIRE(e_offset.unitDimension() == ud); @@ -841,15 +856,24 @@ TEST_CASE( "hzdr_hdf5_sample_content_test", "[serial][hdf5]" ) PatchRecordComponent& e_offset_x = e_offset["x"]; REQUIRE(e_offset_x.unitSI() == 2.599999993753294e-07); - REQUIRE(isSameInteger< uint64_t >( e_offset_x.getDatatype() )); +#if !defined(_MSC_VER) + REQUIRE(e_offset_x.getDatatype() == determineDatatype< uint64_t >()); +#endif + REQUIRE(isSame(e_offset_x.getDatatype(), determineDatatype< uint64_t >())); PatchRecordComponent& e_offset_y = e_offset["y"]; REQUIRE(e_offset_y.unitSI() == 4.429999943501912e-08); - REQUIRE(isSameInteger< uint64_t >( e_offset_y.getDatatype() )); +#if !defined(_MSC_VER) + REQUIRE(e_offset_y.getDatatype() == determineDatatype< uint64_t >()); +#endif + REQUIRE(isSame(e_offset_y.getDatatype(), determineDatatype< uint64_t >())); PatchRecordComponent& e_offset_z = e_offset["z"]; REQUIRE(e_offset_z.unitSI() == 2.599999993753294e-07); - REQUIRE(isSameInteger< uint64_t >( e_offset_z.getDatatype() )); +#if !defined(_MSC_VER) + REQUIRE(e_offset_z.getDatatype() == determineDatatype< uint64_t >()); +#endif + REQUIRE(isSame(e_offset_z.getDatatype(), determineDatatype< uint64_t >())); } catch (no_such_file_error& e) { std::cerr << "HZDR sample not accessible. (" << e.what() << ")\n"; @@ -899,6 +923,22 @@ TEST_CASE( "hdf5_dtype_test", "[serial][hdf5]" ) s.setAttribute("vecLongdouble", std::vector< long double >({0.L, std::numeric_limits::max()})); s.setAttribute("vecString", std::vector< std::string >({"vector", "of", "strings"})); s.setAttribute("bool", true); + short ss = 16; + s.setAttribute("short", ss); + int si = 32; + s.setAttribute("int", si); + long sl = 64; + s.setAttribute("long", sl); + long long sll = 128; + s.setAttribute("longlong", sll); + unsigned short us = 16u; + s.setAttribute("ushort", us); + unsigned int ui = 32u; + s.setAttribute("uint", ui); + unsigned long ul = 64u; + s.setAttribute("ulong", ul); + unsigned long long ull = 128u; + s.setAttribute("ulonglong", ull); } Series s = Series("../samples/dtype_test.h5", AccessType::READ_ONLY); @@ -932,6 +972,26 @@ TEST_CASE( "hdf5_dtype_test", "[serial][hdf5]" ) #endif REQUIRE(s.getAttribute("vecString").get< std::vector< std::string > >() == std::vector< std::string >({"vector", "of", "strings"})); REQUIRE(s.getAttribute("bool").get< bool >() == true); + + // same implementation types (not necessary aliases) detection +#if !defined(_MSC_VER) + REQUIRE(s.getAttribute("short").dtype == Datatype::SHORT); + REQUIRE(s.getAttribute("int").dtype == Datatype::INT); + REQUIRE(s.getAttribute("long").dtype == Datatype::LONG); + REQUIRE(s.getAttribute("longlong").dtype == Datatype::LONGLONG); + REQUIRE(s.getAttribute("ushort").dtype == Datatype::USHORT); + REQUIRE(s.getAttribute("uint").dtype == Datatype::UINT); + REQUIRE(s.getAttribute("ulong").dtype == Datatype::ULONG); + REQUIRE(s.getAttribute("ulonglong").dtype == Datatype::ULONGLONG); +#endif + REQUIRE(isSame(s.getAttribute("short").dtype, Datatype::SHORT)); + REQUIRE(isSame(s.getAttribute("int").dtype, Datatype::INT)); + REQUIRE(isSame(s.getAttribute("long").dtype, Datatype::LONG)); + REQUIRE(isSame(s.getAttribute("longlong").dtype, Datatype::LONGLONG)); + REQUIRE(isSame(s.getAttribute("ushort").dtype, Datatype::USHORT)); + REQUIRE(isSame(s.getAttribute("uint").dtype, Datatype::UINT)); + REQUIRE(isSame(s.getAttribute("ulong").dtype, Datatype::ULONG)); + REQUIRE(isSame(s.getAttribute("ulonglong").dtype, Datatype::ULONGLONG)); } TEST_CASE( "hdf5_write_test", "[serial][hdf5]" ) @@ -1197,7 +1257,10 @@ TEST_CASE( "hdf5_fileBased_write_test", "[serial][hdf5]" ) auto& posOff_x = posOff.at("x"); REQUIRE(posOff_x.unitSI() == 1.); REQUIRE(posOff_x.getExtent() == ext); - REQUIRE(isSameInteger< uint64_t >( posOff_x.getDatatype())); +#if !defined(_MSC_VER) + REQUIRE(posOff_x.getDatatype() == determineDatatype< uint64_t >()); +#endif + REQUIRE(isSame(posOff_x.getDatatype(), determineDatatype< uint64_t >())); auto position = pos_x.loadChunk< double >({0}, {4}); auto position_raw = position.get(); @@ -1746,7 +1809,10 @@ TEST_CASE( "adios1_fileBased_write_test", "[serial][adios1]" ) REQUIRE(species.at("position").at("x").getExtent() == Extent{4}); REQUIRE(species.at("positionOffset").size() == 1); REQUIRE(species.at("positionOffset").count("x") == 1); - REQUIRE(isSameInteger< uint64_t >( species.at("positionOffset").at("x").getDatatype() )); +#if !defined(_MSC_VER) + REQUIRE(species.at("positionOffset").at("x").getDatatype() == determineDatatype< uint64_t >()); +#endif + REQUIRE(isSame(species.at("positionOffset").at("x").getDatatype(), determineDatatype< uint64_t >())); REQUIRE(species.at("positionOffset").at("x").getDimensionality() == 1); REQUIRE(species.at("positionOffset").at("x").getExtent() == Extent{4});