Build fixes

[plopezrios]

Fixes to get cmake to work as advertised and to suppress gfortran 11 warnings

[scemama]

Thanks Pablo!

[q-posev]

Thank you for these fixes @plopezrios !

I have only one suggestion for the CMake part and one comment regarding the use c_long in the Fortran interface.

[plopezrios]

@q-posev @scemama do let me know if you would like any further changes

[q-posev]

@plopezrios thank you for the update! Fine with me.

In the end, I think we should roll back to Fortran-specific integer(8)/(4) and same for the other types because this is more Fortran-ic, namely the user interfacing his code with TREXIO in Fortran will only see the Fortran-like types. The -Wc-binding-type warning will appear upon compilation of trexio_f.f90 module. Otherwise, I guess these warnings will appear when the user links his code with the trexio module. This is based on the assumption that Fortran users typically have arrays declared not in c_int-like types. Let me know if I missed something.

We can probably merge this PR as it is now and then roll back on the master branch.

[plopezrios]

In the end, I think we should roll back to Fortran-specific integer(8)/(4) and same for the other types because this is more Fortran-ic, namely the user interfacing his code with TREXIO in Fortran will only see the Fortran-like types.

According to the final draft of the Fortran 2008 standard (https://j3-fortran.org/doc/year/10/10-007r1.pdf ; see section 15.3 and table 15.2) using c_int64_t etc is the definition of C interoperability, and Fortran compilers should happily match Fortran types with the corresponding C types without complaining. Basically, my understanding is that this should not cause any problems.

The -Wc-binding-type warning will appear upon compilation of trexio_f.f90 module. Otherwise, I guess these warnings will appear when the user links his code with the trexio module.

The Fortran module, by construction (and according to README.md), needs to be compiled (not just linked) by the host project, so these warnings are pushed onto users. And this might be problematic, e.g., NECI by default compiles its pipeline binaries with -Werror (i.e., has a zero-warning policy) so this would have to be worked around there. In any case, I think having zero warnings compiling TREXIO can only be a good thing.

Should you still decide to revert the changes, mind that some of them (changes specifying the c_bool kind, if I remember correctly), also suppressed "GNU extension" warnings from gfortran, which would turn into errors compiling with -std=f2008 and potentially fail to compile with other compilers.

[scemama]

OK. so in that case the best solution is probably to generate 2 different fortran modules: one with integer(8) and one with integer(c_int64_t) and let the users choose which one they want to link in the code. What do you think?

[plopezrios]

OK. so in that case the best solution is probably to generate 2 different fortran modules: one with integer(8) and one with integer(c_int64_t) and let the users choose which one they want to link in the code. What do you think?

Just to be clear, integer(8) and integer(c_int64_t) refer to the same data type, but the latter is guaranteed to be C interoperable in a bind(C) procedure while the former is not.

Regarding the use of integer(8) itself, again according to the standard, the use of constants (like 4 and 8) is not considered portable, and instead one is encouraged to use selected_int_kind(), e.g., integer(selected_int_kind(15)) is the smallest integer kind that can represent all values ranging from -1e15 to +1e15.

[scemama]

Regarding the use of integer(8) itself, again according to the standard, the use of constants (like 4 and 8) is not considered portable, and instead one is encouraged to use selected_int_kind(), e.g., integer(selected_int_kind(15)) is the smallest integer kind that can represent all values ranging from -1e15 to +1e15.

Yes, I think that this move of the Fortran standard was one of the many stupid actions they took!
But I found this on Stackoverflow were they fixed it in Fortran2008:

if you don't want to specify numeric types by the required precision, but instead by the storage that they will use, Fortran 2008 provides a method. reals and integers can be specified by the number of bits of storage after using the ISO_FORTRAN_ENV module, for example, for a 4-byte (32-bit) integer:
use ISO_FORTRAN_ENV
integer (int32) :: MyInt

So I guess we should use the ISO_FORTRAN_ENV then! :-)

[plopezrios]

So I guess we should use the ISO_FORTRAN_ENV then! :-)

Yup, that would make the code portable, but it would still run into the problem that kind constants that do not come from iso_c_binding are not guaranteed to be C interoperable (even if they are the same kind) and compilers are allowed to issue warnings about this.

I don't see that there is anything to lose from having a single Fortran module with the iso_c_binding kind constants? NB, defining a Fortran-kind variable, e.g., integer(int64) :: i, and passing that as an argument declared with a C kind, e.g., integer(c_int64_t), intent(in) :: i, should work without problems.

[scemama]

Yup, that would make the code portable, but it would still run into the problem that kind constants that do not come from iso_c_binding are not guaranteed to be C interoperable (even if they are the same kind) and compilers are allowed to issue warnings about this.

I get it, you are right. We should use c_int64_t instead of int64 in the interface to avoid the compiler complaining.

I don't see that there is anything to lose from having a single Fortran module with the iso_c_binding kind constants? NB, defining a Fortran-kind variable, e.g., integer(int64) :: i, and passing that as an argument declared with a C kind, e.g., integer(c_int64_t), intent(in) :: i, should work without problems.

Suppose that int64 is different from c_int64_t (which is highly improbable...). Is the Fortran compiler going to do the conversion if needed? If it does it, then I agree with your last statement. If not, we will have to make functions that convert the arguments from int64 to c_int64_t before calling the C function, even if 99% of the time it is useless... :-(

[plopezrios]

Suppose that int64 is different from c_int64_t (which is highly improbable...). Is the Fortran compiler going to do the conversion if needed? If it does it, then I agree with your last statement. If not, we will have to make functions that convert the arguments from int64 to c_int64_t before calling the C function, even if 99% of the time it is useless... :-(

My main line of reasoning here is that both iso_c_binding and iso_fortran_env are provided by the Fortran compiler. One would hope that the compiler would agree with itself. But of course reality does have a habit of beating logic.. :-/

[q-posev]

@plopezrios you are right about the C interoperability, I did not know about it. Thank you for the detailed explanation!

Then it seems that usingc_int64_t-like types is our best choice. Let the Fortran compilers do the rest :-)

[scemama]

Then it seems that usingc_int64_t-like types is our best choice. Let the Fortran compilers do the rest :-)

We all agree! I am doing a next PR for that. ready in a few minutes...

[plopezrios]

@plopezrios you are right about the C interoperability, I did not know about it. Thank you for the detailed explanation!

To be fair, I didn't know much about this either until gfortran forced me to go read about it :)