Fortran: simplify H5config_f.inc.cmake to use #cmakedefine directly#6423
Merged
Merged
Conversation
hyoklee
previously approved these changes
May 29, 2026
b0dfb45 to
a6f1ecb
Compare
|
@brtnfld Thank you for tracking down the problem! |
barracuda156
approved these changes
Jun 1, 2026
barracuda156
left a comment
There was a problem hiding this comment.
The patch fixes the build on powerpc-apple-darwin.
a6f1ecb to
c5c1b68
Compare
936f9bb to
b1293fd
Compare
|
@brtnfld Do I need to rebuild with updated patch or the outcome is identical? |
b1293fd to
468e828
Compare
All #cmakedefine01 CMAKE_H5_* blocks used a five-line pattern: #cmakedefine01 CMAKE_H5_HAVE_FOO #if CMAKE_H5_HAVE_FOO == 0 #undef H5_HAVE_FOO #else #define H5_HAVE_FOO #endif This is exactly what #cmakedefine H5_HAVE_FOO does: it emits #define H5_HAVE_FOO (no value) when the CMake variable is truthy and /* #undef H5_HAVE_FOO */ when falsy. The CMAKE_H5_* intermediate variables in fortran/src/CMakeLists.txt were only needed to feed these blocks and are no longer required. Replace all such blocks with #cmakedefine H5_HAVE_FOO, using the H5_* variable directly. MPI_LOGICAL_KIND retains its value via #cmakedefine H5_MPI_LOGICAL_KIND @H5_MPI_LOGICAL_KIND@. This also fixes a real bug: H5_FORTRAN_C_BOOL_IS_UNIQUE was emitted as #define H5_FORTRAN_C_BOOL_IS_UNIQUE 0 when C_BOOL and default LOGICAL are the same kind (e.g. Apple PowerPC ABI). #ifdef only tests whether a macro is defined, not its value, so the guard in H5_test_buildiface.F90 was always true and verify_c_bool was written into tf_gen.F90 regardless, causing an "Ambiguous interfaces" build failure on that platform.
468e828 to
5b044ac
Compare
Collaborator
Author
|
@barracuda156, up to you, I was converting some non-related to H5_FORTRAN_C_BOOL_IS_UNIQUE cases to make them all consistent. It might be good to check once it passes, since we don't test on that particular hardware. I should be done with the updates. |
jhendersonHDF
approved these changes
Jun 1, 2026
Collaborator
Author
|
@barracuda156, all good? It is ready to be merged. |
I will rebuild now. |
|
@brtnfld Yes, all good. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All #cmakedefine01 CMAKE_H5_* blocks used a five-line pattern:
Replace all such blocks with #cmakedefine H5_HAVE_FOO, using the H5_*
variable directly. MPI_LOGICAL_KIND retains its value via
#cmakedefine H5_MPI_LOGICAL_KIND @H5_MPI_LOGICAL_KIND@.
This also fixes #6419: H5_FORTRAN_C_BOOL_IS_UNIQUE was emitted as
#define H5_FORTRAN_C_BOOL_IS_UNIQUE 0 when C_BOOL and default LOGICAL
are the same kind (e.g. Apple PowerPC ABI). #ifdef only tests whether
a macro is defined, not its value, so the guard in H5_test_buildiface.F90
was always true and verify_c_bool was written into tf_gen.F90 regardless,
causing an "Ambiguous interfaces" build failure on that platform.