Potential regression when writing booleans

[ischoegl]

Describe the bug

A recent update (2.7.0?) appears to be have broken the ability of writing boolean attributes; the breakage appears to be limited to the conda package, as I am unable to reproduce it with the main branch.

To Reproduce

Among other data types, we use HighFive to write attributes containing booleans and boolean arrays. We use the following approach:

namespace h5 = HighFive;

enum class H5Boolean {
    FALSE = 0,
    TRUE = 1,
};

h5::EnumType<H5Boolean> create_enum_boolean() {
    return {{"FALSE", H5Boolean::FALSE},
            {"TRUE", H5Boolean::TRUE}};
}

HIGHFIVE_REGISTER_TYPE(H5Boolean, create_enum_boolean)

which is used to eventually either write a scalar bool bValue

            H5Boolean value = bValue ? H5Boolean::TRUE : H5Boolean::FALSE;
            h5::Attribute attr = sub.createAttribute<H5Boolean>(
                name, h5::DataSpace::From(value));
            attr.write(value);

or a vector<bool> bValue

            vector<H5Boolean> values;
            for (auto b : bValue) {
                values.push_back(b ? H5Boolean::TRUE : H5Boolean::FALSE);
            }
            h5::Attribute attr = sub.createAttribute<H5Boolean>(
                name, h5::DataSpace::From(values));
            attr.write(values);

In either case the following error is thrown when attempting to write:

Size of array type 4 != that of memory datatype 1

Expected behavior

Until March 31st, the above code worked for both conda package and compilations using the HighFive git submodule. At this point, it still works for the current main (559bd85), but fails as described above for the conda package. … update: after fixing a clash of function names (see thread below), the main branch no longer works.

Desktop (please complete the following information):

• OS: macOS (same failure also observed on linux)
• Version 2.7.0; conda package only(?)

Additional context

The problem may be limited to the conda package on conda-forge, with the following details

$ conda list
[...]
highfive                  2.7.0                h0747e88_0    conda-forge
[...]

[1uc]

Thank you for reporting the issue. A quick look suggests that it might be related to the fact that we've fixed boolean support, in the same way you seem to have implemented it yourself (if I understand the snippet correctly). See,

HighFive/include/highfive/bits/H5DataType_misc.hpp

Line 209 in c6ffef4

inline EnumType<details::Boolean> create_enum_boolean() {

Note that we've added a function create_enum_boolean which might clash with the one you're mentioning.

This is just a hunch, and I'll look into it more carefully.

[ischoegl]

@1uc ... thanks for the quick response! A clash of names with a newly introduced function does explain things. I have some initial evidence that renaming our function resolves the issue. I will report back once I see what our CI is doing.

[ischoegl]

I verified that renaming the function resolves the issue. Thanks!

[1uc]

Nice to hear it's resolved!

I've checked that we namespace our register invocation:

HighFive/include/highfive/bits/H5DataType_misc.hpp

Line 506 in c6ffef4

HIGHFIVE_REGISTER_TYPE(HighFive::details::Boolean, HighFive::create_enum_boolean)

I suspect (and hope) that fully naming the function when registering it would also work, e.g.,

# Version if `create_enum_boolean` lives in the global namespace.
HIGHFIVE_REGISTER_TYPE(H5Boolean, ::create_enum_boolean)

# Variation if it lies in `app`.
HIGHFIVE_REGISTER_TYPE(H5Boolean, ::app::create_enum_boolean)

[ischoegl]

@1uc ... yes, I can confirm that using the full name ::create_enum_boolean likewise resolves the issue.

Update ... found the root cause for the remaining (now hidden below) - it's a configuration error on our side:

As 2.7.0 removes the need for us to implement `create_enum_boolean` ourselves, I started to experiment with the new capability. One issue I am currently running into is that ```c++ typedef HighFive::details::Boolean H5Boolean; ``` ~works when I use the `conda` package, but it fails with the following error if I check out the git module with version `v2.7.0`: ``` error: no type named 'Boolean' in namespace 'HighFive::details' ``` (I am compiling on macOS)

As an aside, I believe the VERSION file in the root folder is obsolete as it is now set in CMakeLists.txt instead?

[ischoegl]

I am still wondering why create_enum_boolean appears outside of the HighFive namespace? - We are not specifying

using namespace HighFive; // this is nowhere defined in our code

anywhere in our code. Is this a bug after all?

Update: I ran into another failure with 2.7.0 (this time on a GitHub CI runner using MSVC toolset 14.1 - Visual Studio 2017 - that pulled HighFive from conda) see GH Action. Later versions of the MSVC toolset - 14.2 and 14.3 - do work. I believe that the newest standard you’re requiring is C++14 so I believe Visual Studio 2017 should work?

``` c:\miniconda3\envs\test\library\include\highfive\bits/H5Path_traits_misc.hpp(31): error C2248: 'HighFive::Object::Object': cannot access protected member declared in class 'HighFive::Object' c:\miniconda3\envs\test\library\include\highfive\bits/H5Object_misc.hpp(27): note: see declaration of 'HighFive::Object::Object' c:\miniconda3\envs\test\library\include\highfive\H5Object.hpp(53): note: see declaration of 'HighFive::Object' c:\miniconda3\envs\test\library\include\highfive\bits/H5Path_traits_misc.hpp(19): note: while compiling class template member function 'HighFive::PathTraits::PathTraits(void)' C:\Miniconda3\envs\test\Library\include\highfive/H5Attribute.hpp(124): note: see reference to function template instantiation 'HighFive::PathTraits::PathTraits(void)' being compiled C:\Miniconda3\envs\test\Library\include\highfive/H5Attribute.hpp(45): note: see reference to class template instantiation 'HighFive::PathTraits' being compiled C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.16.27023\include\filesystem(2392): note: see reference to class template instantiation 'std::chrono::time_point' being compiled C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.16.27023\include\type_traits(616): note: see reference to class template instantiation 'std::basic_string_view>' being compiled C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.16.27023\include\xstring(2124): note: see reference to class template instantiation 'std::is_convertible>>' being compiled with [ _StringViewIsh=const wchar_t * ] C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.16.27023\include\xstring(2122): note: see reference to variable template 'const bool conjunction_v > >,std::negation > >' being compiled C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.16.27023\include\xstring(2281): note: see reference to alias template instantiation 'std::basic_string,std::allocator>::_Is_string_view_ish<_StringViewIsh>' being compiled with [ _StringViewIsh=const wchar_t * ] ```

[1uc]

First regarding create_enum_boolean. The macro is defined as:

#define HIGHFIVE_REGISTER_TYPE(type, function)                    \
    template <>                                                   \
    inline HighFive::DataType HighFive::create_datatype<type>() { \
        return function();                                        \
    }

When performing:

HIGHFIVE_REGISTER_TYPE(H5Boolean, create_enum_boolean)

it'll expand to:

    template <>                                                   \
    inline HighFive::DataType HighFive::create_datatype<type>() { \
        return create_enum_boolean();                             \
    }

which means that since create_datatype lives inside HighFive, I think it should pick up the version HighFive::create_enum_boolean not ::create_enum_boolean. My hope is that

HIGHFIVE_REGISTER_TYPE(H5Boolean, ::create_enum_boolean)

would protect from this issue since it would expand to:

    template <>                                                   \
    inline HighFive::DataType HighFive::create_datatype<type>() { \
        return ::create_enum_boolean();                           \
    }

which should refer to the function defined in global scope.

What makes me skeptical of my explanation is that you state that it's compiler dependent.

[ischoegl]

Hi @1uc ... thanks for your explanations. Yes, ::create_enum_boolean does prevent the failure.

Regarding some of the other issues I mentioned, I believe I tracked one down on my side, while the compiler dependence is really only limited to MSVC - it likely is related to some of the new issues and PR's that I saw popping up.

[1uc]

Yes, there's a good chance you'll see issues related to macros clashing. However, the CI you linked contained something else too. Can we continue that discussion here #724?