search

HDFGroup / hdf5

Official HDF5® Library Repository
https://www.hdfgroup.org/
Other
619 stars 254 forks source link

Use of `FE_INVALID` breaks WebAssembly builds of HDF5 1.14.5 #4952

Closed LTLA closed 3 weeks ago

LTLA commented 3 weeks ago

Describe the bug

HDF5 1.14.5 cannot compile to WebAssembly as Emscripten does not support the FE_* macros.

Expected behavior

The compilation should succeed.

Platform (please complete the following information)

Additional context

Emscripten fails on the following chunk of code:

https://github.com/HDFGroup/hdf5/blob/6b43197b0817596f47670c6b55d26ff7f86d6bd9/src/H5Tinit_float.c#L611-L613

with:

# ... skipping Cmake logs...
[ 78%] Building C object src/CMakeFiles/hdf5-static.dir/H5Tinit_float.c.o
/home/luna/Code/js/scran.js/extern/hdf5/demo/hdf5-1.14.5/src/H5Tinit_float.c:612:23: error: use of undeclared identifier 'FE_INVALID'
  612 |     if (feclearexcept(FE_INVALID) != 0)
      |                       ^
1 error generated.

This is because many of the FE_* macros are not defined by Emscripten (emscripten-core/emscripten#13678). While I'm not familiar with the intricacies of the C standard, some reading suggests that compliant implementations do not need to define these macros.

Currently I'm modifying the HDF5 source code directly so that the compilation can complete. Would you consider adding an #ifdef FE_INVALID around this block so that it compiles out of the box?

FWIW the MRE is:

# Unzip the downloaded tarball.
tar -xf hdf5-1.14.5.tar.gz

# Avoid problems from a random package.json in a parent directory.
echo "{}" > package.json 

# Assumes you have Emscripten installed, I'm using 3.1.68. Also
# I'm using Cmake 3.27.1, but I don't think it really matters.
emcmake cmake \
    -S hdf5-1.14.5 \
    -B build-1.14.5 \
    -DCMAKE_BUILD_TYPE=Release \
    -DCMAKE_INSTALL_PREFIX=installed \
    -DBUILD_SHARED_LIBS=OFF \
    -DBUILD_TESTING=OFF \
    -DHDF5_BUILD_EXAMPLES=OFF \
    -DHDF5_BUILD_TOOLS=OFF \
    -DHDF5_BUILD_UTILS=OFF \
    -DHDF5_BUILD_CPP_LIB=ON \
    -DHDF5_ENABLE_Z_LIB_SUPPORT=ON \
    -DZLIB_USE_EXTERNAL=OFF \
    -DHDF5_ENABLE_SZIP_SUPPORT=OFF

# Reproduces the error.
cd build-1.14.5
emmake make
bmaranville commented 3 weeks ago

This is also the only usage of an FE_XX macro in the whole HDF5 codebase, as far as I can tell.

derobins commented 3 weeks ago

I think the suggested fix (an #ifdef FE_INVALID block around the error check/clear) is a valid solution.